From: Junio C Hamano <gitster@pobox•com>
To: Paolo Bonzini <bonzini@gnu•org>
Cc: git@vger•kernel.org, Paolo Bonzini <pbonzini@redhat•com>
Subject: Re: [PATCH 2/2] git-am: add --message-id/--no-message-id
Date: Tue, 25 Nov 2014 15:34:02 -0800 [thread overview]
Message-ID: <xmqqbnnusvmd.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1416924056-29993-3-git-send-email-bonzini@gnu.org> (Paolo Bonzini's message of "Tue, 25 Nov 2014 15:00:56 +0100")
Paolo Bonzini <bonzini@gnu•org> writes:
> @@ -371,13 +372,18 @@ split_patches () {
> prec=4
> dotest="$GIT_DIR/rebase-apply"
> sign= utf8=t keep= keepcr= skip= interactive= resolved= rebasing= abort=
> -resolvemsg= resume= scissors= no_inbody_headers=
> +messageid= resolvemsg= resume= scissors= no_inbody_headers=
It is somewhat irritating to read a diff that adds new things to the
beginning of anything (a line, or a block of lines) that lists
things in no particular order, as it just adds cognitive burden.
> git_apply_opt=
> committer_date_is_author_date=
> ignore_date=
> allow_rerere_autoupdate=
> gpg_sign_opt=
>
> +if test "$(git config --bool --get am.messageid)" = true
> +then
> + messageid=t
> +fi
> +
> if test "$(git config --bool --get am.keepcr)" = true
> then
> keepcr=t
> @@ -400,6 +406,10 @@ it will be removed. Please do not use it anymore."
> utf8=t ;; # this is now default
> --no-utf8)
> utf8= ;;
> + -m|--message-id)
> + messageid=t ;;
> + --no-message-id)
> + messageid=f ;;
This one, taken together with these two hunks ...
> -k|--keep)
> keep=t ;;
> --keep-non-patch)
> @@ -567,6 +577,7 @@ Use \"git am --abort\" to remove it.")"
> echo "$sign" >"$dotest/sign"
> echo "$utf8" >"$dotest/utf8"
> echo "$keep" >"$dotest/keep"
> + echo "$messageid" >"$dotest/messageid"
> echo "$scissors" >"$dotest/scissors"
> echo "$no_inbody_headers" >"$dotest/no_inbody_headers"
> echo "$GIT_QUIET" >"$dotest/quiet"
> @@ -621,6 +632,12 @@ b)
> *)
> keep= ;;
> esac
> +case "$(cat "$dotest/messageid")" in
> +t)
> + messageid=-m ;;
> +f)
> + messageid= ;;
> +esac
... makes the result look questionable. The variable is initialized
to empty; when it is written out to $dotest/messageid and later read
back here, that empty value is not covered by this case statement.
Perhaps clearing messageid= upon seeing "--no-message-id" and using
"'t' or empty" makes the code a bit easier to follow? I dunno.
next prev parent reply other threads:[~2014-11-25 23:34 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-25 14:00 [PATCH 0/2] git-am: add --message-id/--no-message-id options Paolo Bonzini
2014-11-25 14:00 ` [PATCH 1/2] git-mailinfo: add --message-id Paolo Bonzini
2014-11-25 14:00 ` [PATCH 2/2] git-am: add --message-id/--no-message-id Paolo Bonzini
2014-11-25 23:34 ` Junio C Hamano [this message]
2014-11-26 7:06 ` Paolo Bonzini
2014-11-25 16:27 ` [PATCH 0/2] git-am: add --message-id/--no-message-id options Christian Couder
2014-11-25 17:01 ` Paolo Bonzini
2014-11-25 21:21 ` Christian Couder
2014-11-26 9:07 ` Paolo Bonzini
2014-11-27 0:23 ` Christian Couder
2014-11-25 18:33 ` Junio C Hamano
2014-11-25 19:16 ` Paolo Bonzini
2014-11-25 20:05 ` 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=xmqqbnnusvmd.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=bonzini@gnu$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=pbonzini@redhat$(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