git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] log: add %S option (like --source) to log --format

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 28, 2018, 18:14 UTC
Message-ID
<xmqqd0plplhe.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<5dfd92d1-2e87-3006-1630-a33794b6066b@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
Show 11 quoted lines
>> +++ b/Documentation/pretty-formats.txt
>> @@ -134,6 +134,8 @@ The placeholders are:
>>   - '%cI': committer date, strict ISO 8601 format
>>   - '%d': ref names, like the --decorate option of linkgit:git-log[1]
>>   - '%D': ref names without the " (", ")" wrapping.
>> +- '%S': ref name given on the command line by which the commit was reached
>> +  (like `git log --source`), only works with `git log`
>
> This "only works with `git log`" made me think about what would happen
> with `git rev-list --pretty=format:"%h %S"` and the answer (on my
> machine) was a segfault.
That's a bad one X-<.
Show 17 quoted lines
>> +		slot = revision_sources_at(c->pretty_ctx->rev->sources, commit);
>> +		if (slot && *slot) {
> I'm not sure this check for 'slot' being non-null is necessary, as we
> would already get a failure in the commit-slab code (for
> revision_sources_at()) if the slab is not initialized.
>> +			strbuf_addstr(sb, *slot);
>> +			return 1;
>> +		} else {
>> +			die(_("failed to get info for %%S"));
>
> Here, you die() when you fail to get a slot but above you return 0
> when the sources are not initialized.
>
> I don't see another use of die() in this method. Is that the right way
> to handle failure here? (I'm legitimately asking because I have
> over-used 'die()' in the past and am still unclear on when it is
> appropriate.)

This is definitely a bad one, too. If '%d' cannot find decoration, it would not "die".

Thanks.
Previous: Derrick StoleeNext: Issac Trotts
Message 6 of 7 in “log: add %S option (like --source) to log --format”
  1. log: add %S option (like --source) to log --formatissac.trotts@gmail.com, Dec 19, 2018
  2. Issac TrottsDec 21, 2018
  3. Jeff KingDec 22, 2018
  4. Issac TrottsDec 25, 2018
  5. Derrick StoleeDec 27, 2018
  6. Junio C HamanoDec 28, 2018
  7. Issac TrottsDec 31, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.