2015-12-01 13:53 GMT+01:00 Michael Paquier <michael.paqu...@gmail.com>:
> On Tue, Dec 1, 2015 at 11:46 AM, Pavel Stehule <pavel.steh...@gmail.com> > wrote: > > 2015-11-30 15:17 GMT+01:00 Michael Paquier <michael.paqu...@gmail.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 >
