On Sat, Aug 26, 2023 at 02:09:49PM +0200, JWM van den Heuvel wrote:
The previous mail was me sending -- instead of saving -- a draft. My bad.

thought so. no worries.

The perl scripts are a
work of art and make me want to punch something.

compliment accepted. :'-D

A nice future refactoring project. Looks good, but given the amount of
feedback my contributions require, not efficient if I take it up ;)

that depends on whose perspective you're assuming here.
i'm likely not going to get it done anytime soon. pushing patches on me prompts me into action, so it's good for the project (not so much for my other projects, but the trade-off isn't that bad, as i'm actually much more efficient at reviewing than hacking myself).
so if you think it would be a good use of *your* time, then go ahead.

> + if (cfile->cmd && !strcasecmp( cfile->cmd, "IncludeCmd" > )) {
>     +              if (cfile->include_fp)
>     +                      conf_error( cfile, "nested IncludeCmd\n" );
>     +              else
>     +                      include_cmd_popen( cfile, cfile->val );
>     +      }

wouldn't just moving the body into the conditional below work?
of course that would do the excess-argument check only after doing the
popen, but that's consistent with how it works for other options.
otoh, the side effects of instantly executing a potentially malformed
command may be somewhat more grave than when processing other options.
but handlign this properly would actually require a separate check
anyway, as then we need to actually skip the command.

The problem with doing it after the popen is that `conf_print_loc`
will then show an error as if it is in the included output while it
is in the file itself.

right.

Moving it would mean copying the excess token
detection code, which is not worth it IMHO.

i implied that you need to do that anyway to handle things properly. also note that you'd save the duplicated check for the command.

but looking at it again, duplication probably isn't even necessary: you can factor out check_excess_tokens() and make it also return a value.

the first patch is now Perfect (TM), so no need to re-send it.

regards


_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel

Reply via email to