From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Git List Mailing <git@vger.kernel.org>
Subject: Re: Silly 'git am' UI issue
Date: Thu, 03 Mar 2022 19:51:19 -0800 [thread overview]
Message-ID: <xmqq1qzi4bpk.fsf@gitster.g> (raw)
In-Reply-To: CAHk-=wjQP8PwYZMYUhKu3s9wMhE2yDqdAO_OKagqXuvPrHRyAQ@mail.gmail.com
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Thu, Mar 3, 2022 at 1:58 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I wonder if something like this would have helped.
>
> That would certainly have helped, yes.
>
> But I think we can do better.
>
> How about just parsing the "--whitespace" option in 'git am' before
> passing it through?
>
> Something like the attached patch seems to work for me.
>
> With this one, I simply get
>
> $ git am --whitespace 0001-Dummy.patch
> error: unrecognized whitespace option '0001-Dummy.patch'
>
> when I make the mistake of not giving that whitespace argument.
>
> I'm not claiming this is extensively tested, but I did *some* testing
> on it, and it's not a complicated patch.
>
> Linus
Yup, reusing what apply.c already uses gives reviewers a solid
assurance. It already is named appropriately for a global
namespace, so the patch text, as far as I can see, looks good
enough.
> +static int parse_opt_whitespace(const struct option *opt, const char *arg, int unset)
> +{
> + struct apply_state dummy = { };
It is rare to see a completely empty initializer in this codebase, I
think.
> + if (parse_whitespace_option(&dummy, arg))
> + return -1;
> +
> + return parse_opt_passthru_argv(opt, arg, unset);
> +}
Looks good.
> static int git_am_config(const char *k, const char *v, void *cb)
> {
> int status;
> @@ -2355,9 +2365,9 @@ int cmd_am(int argc, const char **argv, const char *prefix)
> OPT_CALLBACK_F(0, "quoted-cr", &state.quoted_cr, N_("action"),
> N_("pass it through git-mailinfo"),
> PARSE_OPT_NONEG, am_option_parse_quoted_cr),
> - OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, N_("action"),
> + OPT_CALLBACK(0, "whitespace", &state.git_apply_opts, N_("action"),
> N_("pass it through git-apply"),
> - 0),
> + parse_opt_whitespace),
> OPT_PASSTHRU_ARGV(0, "ignore-space-change", &state.git_apply_opts, NULL,
> N_("pass it through git-apply"),
> PARSE_OPT_NOARG),
next prev parent reply other threads:[~2022-03-04 3:51 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-03 20:31 Silly 'git am' UI issue Linus Torvalds
2022-03-03 21:58 ` Junio C Hamano
2022-03-04 0:42 ` Linus Torvalds
2022-03-04 3:51 ` Junio C Hamano [this message]
2022-03-04 7:01 ` Ævar Arnfjörð Bjarmason
2022-03-04 7:14 ` Ævar Arnfjörð Bjarmason
2022-03-04 18:50 ` Linus Torvalds
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=xmqq1qzi4bpk.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.