From: Jeff Hostetler <git@jeffhostetler.com>
Date: Fri, 05 Aug 2016 18:11:16 GMT
Subject: Re: [PATCH v4 6/8] status: print branch info with --porcelain=v2 --branch
Message-ID: <57A4D6C4.3010501@jeffhostetler.com>
In-Reply-To: <xmqqd1lnxi99.fsf@gitster.mtv.corp.google.com>



On 08/05/2016 01:01 PM, Junio C Hamano wrote:
> Jeff Hostetler <git@jeffhostetler.com> writes:
>
>>   /*
>> + * Print branch information for porcelain v2 output.  These lines
>> + * are printed when the '--branch' parameter is given.
>> + *
>> + *    # branch.oid <commit><eol>
>> + *    # branch.head <head><eol>
>
> Just bikeshedding, but ...
>
>> +	if (!s->branch)
>> +		fprintf(s->fp, "# branch.head %s%c", "(unknown)", eol);
>> +	else {
>> +		if (!strcmp(s->branch, "HEAD")) {
>> +			fprintf(s->fp, "# branch.head %s%c", "(detached)", eol);
>> +
>> +			if (state.rebase_in_progress || state.rebase_interactive_in_progress)
>> +				branch_name = state.onto;
>> +			else if (state.detached_from)
>> +				branch_name = state.detached_from;
>> +			else
>> +				branch_name = "";
>> +		} else {
>> +			branch_name = NULL;
>> +			skip_prefix(s->branch, "refs/heads/", &branch_name);
>> +
>> +			fprintf(s->fp, "# branch.head %s%c", branch_name, eol);
>
> ... given that we are showing branch name, perhaps "branch.name"
> instead of "branch.head" is more appropriate?

Either way is fine with me.  I just saw your comments on
commit v4-7/8. And leaving it as is is fine.

>
> I wondered if "# " prefix before these lines is useful, by the way,
> and initially thought that the fact that these lines begin with
> "branch." and not with the "1/2/u $key" sufficient clue for whoever
> reads them, but the reader can tell which kind of record it is by
> reading the first two characters of each line (i.e. if "# " that is
> not the usual "change info for a single file"), so it is actually a
> good idea.

Yes, I used the "#" prefix to indicate a header line so that
parsers can always look at the first character and decide.

I set it up so the "--branch" argument creates "# branch.*" headers.

In my first patch series, I included worktree state information
(such as in-a-rebase and the rebase step counts), but it was thought
that that should be an independent effort. So, using this model,
if we later do add a "--state" argument, it would create "# state.*"
headers.  And we would not have to change the --porcelain=v2 version
number.


Jeff