Re: [PATCH v2 5/8] status: print per-file porcelain v2 status data

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2016, 20:23 UTC
Message-ID: <xmqqwpk94gbu.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1469474750-49075-6-git-send-email-jeffhost@microsoft.com>

Jeff Hostetler <jeffhost@microsoft.com> writes:

> +static void wt_porcelain_v2_print(struct wt_status *s);
> +

There is no point in this forward declaration, if you just place the implementation of these functions here, no?

Show 9 quoted lines

> +/*
> + * Print porcelain v2 info for tracked entries with changes.
> + */
> +static void wt_porcelain_v2_print_changed_entry(
> +	struct string_list_item *it,
> +	struct wt_status *s)
> +{
> +...
> +	fprintf(s->fp, "%c %s %s %06o %06o %06o %s %s R%d %s",

It is misleading to always say R in the output when there is no rename, isn't it?

> +	 * Note that this is a last-one-wins for each the individual
> +	 * stage [123] columns in the event of multiple cache rows
> +	 * for a stage.

Just FYI, the usual lingo we use for that is "multiple cache entries for the same stage", I would think.

Show 17 quoted lines

> +	 */
> +	memset(stages, 0, sizeof(stages));
> +	sum = 0;
> +	pos = cache_name_pos(it->string, strlen(it->string));
> +	assert(pos < 0);
> +	pos = -pos-1;
> +	while (pos < active_nr) {
> +		ce = active_cache[pos++];
> +		stage = ce_stage(ce);
> +		if (strcmp(ce->name, it->string) || !stage)
> +			break;
> +		stages[stage - 1].mode = ce->ce_mode;
> +		hashcpy(stages[stage - 1].oid.hash, ce->sha1);
> +		sum++;
> +	}
> +	if (!sum)
> +		die("BUG: unmerged entry without any stages");

Hmm, we seem to already have d->stagemask; if you call that variable "sum" anyway, perhaps its computation can be more like

	sum |= 1 << (stage - 1);

so that you can compare it with d->stagemask for this sanity check?