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.
