From: Andreas Ericsson <ae@op5.se>
Date: Thu, 02 Oct 2008 05:40:28 GMT
Subject: Re: [PATCH] git commit: Repaint the output format bikeshed (again)
Message-ID: <48E45ECC.8070104@op5.se>
In-Reply-To: <20081001223125.GA25267@coredump.intra.peff.net>

Jeff King wrote:
> On Wed, Oct 01, 2008 at 06:06:04PM -0400, Jeff King wrote:
> 
>> I think I still like your other proposal:
>>
>>   [branch] created b930c4a: "i386: Snib the sprock"
> 
> And here is the patch, since it was sitting uncommitted in my working
> tree. Feel free to ignore.
> 
> BTW, we should apply _something_ since what is currently in next has a
> bug: it lacks a space between "DETACHED commit" and the hash:
> 
>   Created DETACHED commit4fde0d0 (subject line)
> 
> -- >8 --
> reformat informational commit message
> 
> When committing, we print a message like:
> 
>   Created [DETACHED commit] <hash> (<subject>) on <branch>
> 
> The most useful bit of information there (besides the
> detached status, if it is present) is which branch you made
> the commit on. However,  it is sometimes hard to see because
> the subject dominates the line.
> 
> Instead, let's put the most useful information (detached
> status and commit branch) on the far left, with the subject
> (which is least likely to be interesting) on the far right.
> 
> We'll use brackets to offset the branch name so the line is
> not mistaken for an error line of the form "program: some
> sort of error". E.g.,:
> 
>   [jk/bikeshed] created bd8098f: "reformat informational commit message"
> ---

No sign-off.

>  builtin-commit.c |   37 ++++++++++---------------------------
>  1 files changed, 10 insertions(+), 27 deletions(-)
> 
> diff --git a/builtin-commit.c b/builtin-commit.c
> index e4e1448..7a66e5a 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -878,35 +878,13 @@ int cmd_status(int argc, const char **argv, const char *prefix)
>  	return commitable ? 0 : 1;
>  }
>  
> -static char *get_commit_format_string(void)
> -{
> -	unsigned char sha[20];
> -	const char *head = resolve_ref("HEAD", sha, 0, NULL);
> -	struct strbuf buf = STRBUF_INIT;
> -
> -	/* use shouty-caps if we're on detached HEAD */
> -	strbuf_addf(&buf, "format:%s", strcmp("HEAD", head) ? "" : "DETACHED commit");
> -	strbuf_addstr(&buf, "%h (%s)");
> -
> -	if (!prefixcmp(head, "refs/heads/")) {
> -		const char *cp;
> -		strbuf_addstr(&buf, " on ");
> -		for (cp = head + 11; *cp; cp++) {
> -			if (*cp == '%')
> -				strbuf_addstr(&buf, "%x25");
> -			else
> -				strbuf_addch(&buf, *cp);
> -		}
> -	}
> -
> -	return strbuf_detach(&buf, NULL);
> -}
> -
>  static void print_summary(const char *prefix, const unsigned char *sha1)
>  {
>  	struct rev_info rev;
>  	struct commit *commit;
> -	char *format = get_commit_format_string();
> +	static const char *format = "format:%h: \"%s\"";
> +	unsigned char junk_sha1[20];
> +	const char *head = resolve_ref("HEAD", junk_sha1, 0, NULL);
>  
>  	commit = lookup_commit(sha1);
>  	if (!commit)
> @@ -931,7 +909,13 @@ static void print_summary(const char *prefix, const unsigned char *sha1)
>  	rev.diffopt.break_opt = 0;
>  	diff_setup_done(&rev.diffopt);
>  
> -	printf("Created %s", initial_commit ? "root-commit " : "");
> +	printf("[%s%s]: created ",
> +		!prefixcmp(head, "refs/heads/") ?
> +			head + 11 :
> +			!strcmp(head, "HEAD") ?
> +				"detached HEAD" :
> +				head,
> +		initial_commit ? " (root-commit)" : "");
>  

Personally, I'm not overly fond of things like
   something ? yay : nay_but_try ? worked_now : still_no_go
since I find them hard to read without thinking a lot.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231