Re: PATCH 6/6] archive: convert,to parse_options() [was: [PATCH 5/6] archive: allow --exec and --remote without equal sign]

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Junio C Hamano schrieb:
>>  archive.c |  110 +++++++++++++++++++++++++++++++++++++-----------------------
>>  1 files changed, 68 insertions(+), 42 deletions(-)
> 
> Hmph, somewhat dubious.
> 
> The real point of parse-options was to make the code smaller, easier to
> maintain and command line handling more consistent.  At least this patch
> seems to fail on the two out of three counts.

Well, if we hid away the compression level handling in a macro defined
in parse-options.h, we could save sixteen lines of code.  The patch
makes the four modes of running archive more explicit, adding three
usage lines.  Three empty lines are added -- they don't really increase
the code's size.

Handling --exec and --remote takes six lines; we didn't do that before
at this place, but have to now, since we want them to show up in the
usage.  We have to handle --no-format and --no-prefix, which adds four
lines.

So I don't think the bigger size make this patch dubious, but of course
I'm biased.  Disallowing --no-format (using a new OPT_STRING_NONEG?) and
adding an OPT__COMPRESSION helper might be a good idea (reducing line
count in archive.c by seventeen).

Having parse-options provide a way to make --exec and --remote appear in
the usage but to reject them (OPT_UNKNOWN?) is a bit too strange, though.

> All of the other patches made obvious sense to me and are queued for -rc1
> but I'd like to backburner this one.

Fair enough.

René