v25 - PQExpBuffer on gather_boolean_expression() - convenience/clarity functions: ignore_slash_option(), ignore_2_slash_options(), ignore_slash_line() - remove inaccurate test of variable expansion in a false block - added kitchen-sink test of parsing slash commands in a false block - removed spurious file that shouldn't have been in v24 - removed any potential free(NULL) calls *that I introduced*, others remain from master branch
NOT done: - grouping all branching commands into one function - can be done in a later patch for clarity - combining _ef / _ev or _sf / _sv - can be done in a later patch for clarity On Fri, Mar 24, 2017 at 4:33 PM, Corey Huinker <corey.huin...@gmail.com> wrote: > On Fri, Mar 24, 2017 at 4:10 PM, Fabien COELHO <coe...@cri.ensmp.fr> > wrote: > >> >> Hello Corey, >> >> I wished for the same thing, happy to use one if it is made known to me. >>> I pulled that pattern from somewhere else in the code, and given that the >>> max number of args for a command is around 4, I'm not too worried about >>> scaling. >>> >> >> If there are expressions one day like pgbench, the number of arguments >> becomes arbitrary. Have you looked at PQExpBuffer? > > > I will look into it. > > >> >>>> There seems to be pattern repetition for _ev _ef and _sf _sv. Would it >>>> make sense to create a function instead of keeping the initial >>>> copy-paste? >>>> >>> >>> Yes, and a few things like that, but I wanted this patch to keep as much >>> code as-is as possible. >>> >> >> If you put the generic function at the same place, one would be more or >> less kept and the other would be just removed? > > >> "git diff --patience -w" gives a rather convenient output for looking at >> the patch. > > > Good to know about that option. > > As for a function for digested ignored slash options, it seems like I can > disregard the true/false value of the "semicolon" parameter. Is that > correct? > > >> I would suggest to put together all if-related backslash command, so that >>>> the stack management is all in one function instead of 4. >>>> >>> >>> I recognize the urge to group them together, but would there be any >>> actual >>> code sharing between them? Wouldn't I be either re-checking the string >>> "cmd" again, or otherwise setting an enum that I immediately re-check >>> inside the all_branching_commands() function? >>> >> >> I do not see that as a significant issue, especially compared to the >> benefit of having the automaton transition management in a single place. > > > I'm still struggling to see how this would add any clarity to the code > beyond what I can achieve by clustering the exec_command_(if/elif/else/endif) > near one another. > > > >
0001.if_endif.v25.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
