From: "René Scharfe" <rene.scharfe@lsrfire•ath.cx>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org
Subject: Re: PATCH 6/6] archive: convert,to parse_options() [was: [PATCH 5/6] archive: allow --exec and --remote without equal sign]
Date: Sat, 26 Jul 2008 09:09:24 +0200 [thread overview]
Message-ID: <488ACDA4.4010404@lsrfire.ath.cx> (raw)
In-Reply-To: <7vmyk5pjsy.fsf@gitster.siamese.dyndns.org>
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é
next prev parent reply other threads:[~2008-07-26 7:10 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-25 10:41 [PATCH 1/6] archive: add write_archive() Rene Scharfe
2008-07-25 10:41 ` [PATCH 2/6] archive: move parameter parsing code to archive.c Rene Scharfe
2008-07-25 10:41 ` [PATCH 3/6] archive: define MAX_ARGS where it's needed Rene Scharfe
2008-07-25 10:41 ` [PATCH 4/6] archive: declare struct archiver " Rene Scharfe
2008-07-25 10:41 ` [PATCH 5/6] archive: allow --exec and --remote without equal sign Rene Scharfe
2008-07-25 10:41 ` Rene Scharfe
2008-07-25 10:54 ` René Scharfe
2008-07-26 0:31 ` Junio C Hamano
2008-07-26 7:09 ` René Scharfe [this message]
2008-07-26 0:28 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=488ACDA4.4010404@lsrfire.ath.cx \
--to=rene.scharfe@lsrfire$(echo .)ath.cx \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox