Re: [PATCH v2 1/3] am: add --show-current-patch
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 31, 2018, 22:59 UTC
- Message-ID
- <xmqqpo5pr5wr.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <CAPig+cR24=0_NPekYaF+oG9OovDkv1Et-RpNvAHqE7Qd7g7QQA@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
Show 23 quoted lines
> On Wed, Jan 31, 2018 at 4:30 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
>> around in there, which is not a good thing. With this, the user does
>> not have to keep the path around somewhere (because after a couple of
>> commands, the path may be out of scrollback buffer) when they need to
>> look at the patch.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>> diff --git a/builtin/am.c b/builtin/am.c
>> @@ -2121,6 +2120,22 @@ static void am_abort(struct am_state *state)
>> +static int show_patch(struct am_state *state)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> + int len;
>> +
>> + len = strbuf_read_file(&sb, am_path(state, msgnum(state)), 0);
>> + if (len < 0)
>> + die_errno(_("failed to read '%s'"),
>> + am_path(state, msgnum(state)));
>
> Isn't this am_path() invocation inside die_errno() likely to clobber
> the 'errno' from strbuf_read_file() which you want to be reporting?True. After coming up with the pathname to the current patch file, we are going to exit without ever calling am_path(), or underlying get_pathname() via mkpath(), again before exiting anyway, so perhaps it is sufficient to just do an am_path() just once upfront, feed it to strbuf_read_file() and also to die_errno().