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

Re: [PATCH] Use "" instead of "<unknown>" for placeholders

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Sep 25, 2007, 10:25 UTC
Message-ID
<Pine.LNX.4.64.0709251120120.28395@racer.site>
In-Reply-To
<20070925094320.GN22869@mageo.cz>
Hi,
On Tue, 25 Sep 2007, Michal Vitecek wrote:
Show 31 quoted lines
>  Hello again,
> 
> Michal Vitecek wrote:
> > Junio C Hamano wrote:
> >>>>>>> I made it because I want to use my own pretty format which currently 
> >>>>>>> only allows '%s' for subject and '%b' for body. But '%b' is 
> >>>>>>> substituted with <undefined> if the body is "missing" which I 
> >>>>>>> obviously don't like :)
> >>>>>> Then you should fix %b not to show "<undefined>".
> >>>>>  I'll do it if it is okay. Shall I do the same for the other
> >>>>>  placeholders as well?
> >>>> Yeah.  Don't know why I did it that way.
> >>>  Here comes the big patch :)
> >> Now, this breaks t6006 which needs this patch.
> >
> > Oops - I'm sorry about that. I ran the test suite (1.5.3.1) but it failed 
> > in 2 tests before the patch and in 2 tests after it so I considered it 
> > okay.
> >
> >> Looking at this patch, I am not sure if your change is really a
> >> desirable one --- shouldn't it be removing the line itself, not
> >> just <unknown> token?
> >
> > This sounds as the best solution. I'll look into it. Thanks for your time.
> 
>  Here comes the patch. I hope it will be ok this time :) Thanks.
> 
>  Don't use "<unknown>" for unknown values of placeholders and suppress
>  printing of empty user formats.
> 
> ---

Please move the discussion which should not be in the commit message _after_ the "---".

Show 11 quoted lines
> diff --git a/builtin-rev-list.c b/builtin-rev-list.c
> index 3894633..1de981d 100644
> --- a/builtin-rev-list.c
> +++ b/builtin-rev-list.c
> @@ -85,7 +85,8 @@ static void show_commit(struct commit *commit)
>  		pretty_print_commit(revs.commit_format, commit, ~0,
>  				    &buf, &buflen,
>  				    revs.abbrev, NULL, NULL, revs.date_mode);
> -		printf("%s%c", buf, hdr_termination);
> +                if (strlen(buf))
> +                    printf("%s%c", buf, hdr_termination);
We use tabs for indentation, not spaces.

Also, instead of the expensive "strlen(buf)", you rather want to check "if (*buf)".

Show 11 quoted lines
> diff --git a/log-tree.c b/log-tree.c
> index a642371..5653332 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -298,7 +299,8 @@ void show_log(struct rev_info *opt, const char *sep)
>  	if (opt->show_log_size)
>  		printf("log size %i\n", len);
>  
> -	printf("%s%s%s", msgbuf, extra, sep);
> +        if (strlen(msgbuf))
> +            printf("%s%s%s", msgbuf, extra, sep);
Again, "if (*msgbuf)" is way more efficient.

Ciao, Dscho

Previous: Michal VitecekNext: Michal Vitecek
Message 10 of 12 in “Added a new placeholder '%cm' for full commit message”
  1. Added a new placeholder '%cm' for full commit messageMichal Vitecek, Sep 21, 2007
  2. Johannes SchindelinSep 21, 2007
  3. Michal VitecekSep 21, 2007
  4. Johannes SchindelinSep 21, 2007
  5. Use "" instead of "<unknown>" for placeholders [was Re: [PATCH] Added a new placeholder '%cm' for full commit message]Michal Vitecek, Sep 21, 2007
  6. Junio C HamanoSep 21, 2007
  7. Johannes SchindelinSep 21, 2007
  8. Michal VitecekSep 22, 2007
  9. Michal VitecekSep 25, 2007
  10. Johannes SchindelinSep 25, 2007
  11. Michal VitecekSep 25, 2007
  12. Johannes SchindelinSep 25, 2007

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.