Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2

3 messages, 3 authors, 2016-08-07 · open the first message on its own page

Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2

From: Junio C Hamano <hidden>
Date: 2016-08-06 20:05:21

Jeff Hostetler [off-list ref] writes:
 		if (ce_stage(ce)) {
 			d->index_status = DIFF_STATUS_UNMERGED;
 			d->stagemask |= (1 << (ce_stage(ce) - 1));
+			/*
+			 * Don't bother setting {mode,oid}_{head,index} since the print
+			 * code will output the stage values directly and not use the
+			 * values in these fields.
+			 */
 		}
-		else
+		else {
 			d->index_status = DIFF_STATUS_ADDED;
+			/* Leave {mode,oid}_head zero for adds. */
+			d->mode_index = ce->ce_mode;
+			hashcpy(d->oid_index.hash, ce->sha1);
+		}
Not a big deal (no need to resend for this one alone), but let's
make the above properly formatted, i.e.

		if (ce_stage(ce)) {
                	...
		} else {
                	...
		}                        

Thanks.

Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2

From: Johannes Schindelin <hidden>
Date: 2016-08-07 08:34:40

Hi Junio,

On Fri, 5 Aug 2016, Junio C Hamano wrote:
Jeff Hostetler [off-list ref] writes:
quoted
 		if (ce_stage(ce)) {
 			d->index_status = DIFF_STATUS_UNMERGED;
 			d->stagemask |= (1 << (ce_stage(ce) - 1));
+			/*
+			 * Don't bother setting {mode,oid}_{head,index} since the print
+			 * code will output the stage values directly and not use the
+			 * values in these fields.
+			 */
 		}
-		else
+		else {
 			d->index_status = DIFF_STATUS_ADDED;
+			/* Leave {mode,oid}_head zero for adds. */
+			d->mode_index = ce->ce_mode;
+			hashcpy(d->oid_index.hash, ce->sha1);
+		}
Not a big deal (no need to resend for this one alone), but let's
make the above properly formatted, i.e.

		if (ce_stage(ce)) {
                	...
		} else {
                	...
		}                        
Do I understand correctly that your objections is against having the curly
brace before the "else" on its own line?

If so, when did our coding style change? I vividly remember that we
strongly favored putting the "else" on a new line after a closing brace,
to make diffs nicer in case the braces were removed or added.

BTW your suggestion has 24 extra spaces after the final closing brace ;-)

Ciao,
Dscho

Re: [PATCH v5 4/9] status: collect per-file data for --porcelain=v2

From: Eric Wong <hidden>
Date: 2016-08-07 22:29:16

Johannes Schindelin [off-list ref] wrote:
On Fri, 5 Aug 2016, Junio C Hamano wrote:
quoted
Jeff Hostetler [off-list ref] writes:
quoted
 		}
-		else
+		else {
 			d->index_status = DIFF_STATUS_ADDED;
+			/* Leave {mode,oid}_head zero for adds. */
+			d->mode_index = ce->ce_mode;
+			hashcpy(d->oid_index.hash, ce->sha1);
+		}
Not a big deal (no need to resend for this one alone), but let's
make the above properly formatted, i.e.

		if (ce_stage(ce)) {
                	...
		} else {
                	...
		}                        
Do I understand correctly that your objections is against having the curly
brace before the "else" on its own line?

If so, when did our coding style change? I vividly remember that we
strongly favored putting the "else" on a new line after a closing brace,
to make diffs nicer in case the braces were removed or added.
AFAIK, Linux kernel CodingStyle has always been what Junio
suggested (just w/o the trailing spaces :),
and we inherit from that.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/CodingStyle
BTW your suggestion has 24 extra spaces after the final closing brace ;-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help