Re: proposal: multiple psql option -c

2015-12-01 13:53 GMT+01:00 Michael Paquier <michael(dot)paquier(at)gmail(dot)com>:

> 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,
>

yes, it is looking well

Thank you

Pavel

> --
> Michael
>