Re: proposal: multiple psql option -c

On Tue, Dec 1, 2015 at 11:46 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> 2015-11-30 15:17 GMT+01:00 Michael Paquier <michael(dot)paquier(at)gmail(dot)com>:
>> Removing some items from the list of potential actions and creating a
>> new sublist listing action types is a bit weird. Why not grouping them
>> together and allow for example -l as well in the list of things that
>> is considered as a repeatable action? It seems to me that we could
>> simplify the code this way, and instead of ACT_NOTHING we could check
>> if the list of actions is empty or not.
>
>
> fixed

Thanks for the patch. I have to admit that adding ACT_LIST_DB in the
list of actions was not actually a good idea. It makes the patch
uglier.

Except that, I just had an in-depth look at this patch, and there are
a couple of things that looked strange:
- ACT_LIST_DB would live better if removed from the list of actions
and be used as a separate, independent option. My previous suggestion
was unadapted. Sorry.
- There is not much meaning to have simple_action_list_append and all
its structures in common.c and common.h. Its use is limited in
startup.c (this code is basically a duplicate of dumputils.c still
things are rather different, justifying the duplication) and
centralized around parse_psql_options.
- use_stdin is not necessary. It is sufficient to rely on actions.head
== NULL instead.
- The documentation is pretty clear. That's nice.
Attached is a patch implementing those suggestions. This simplifies
the code without changing its usefulness. If you are fine with those
changes I will switch this patch as ready for committer.
Regards,
--
Michael