Re: [PATCH v2 2/3] for-each-ref: add 'is-base' token

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +is-base:<committish>::
> +	In at most one row, `(<committish>)` will appear to indicate the ref
> +	that is most likely the ref used as a starting point for the branch
> +	that produced `<committish>`. This choice is made using a heuristic:
> +	choose the ref that minimizes the number of commits in the
> +	first-parent history of `<committish>` and not in the first-parent
> +	history of the ref.

Very nicely described.  

Giving the end-user oriented "purpose/meaning" first makes it easier
to understand for readers when they want to use it, and giving the
heuristics to compute the result (and the example) next allows them
to verify that the feature matches what they are looking for.


> @@ -2475,6 +2495,16 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
>  				v->s = xstrdup("");
>  			}
>  			continue;
> +		} else if (atom_type == ATOM_ISBASE) {
> +			if (ref->is_base && ref->is_base[is_base_atoms]) {
> +				v->s = xstrfmt("(%s)", ref->is_base[is_base_atoms]);
> +				free(ref->is_base[is_base_atoms]);
> +			} else {
> +				/* Not a commit. */

This is unexpected.  I thought that most of the branches except at
most one that gets annotated with "Yeah, this is forked from branch
B" would take the "else" side.  They are still commits, no?

> +				v->s = xstrdup("");
> +			}
> +			is_base_atoms++;
> +			continue;
>  		} else
>  			continue;
>  
> @@ -2876,6 +2906,7 @@ static void free_array_item(struct ref_array_item *item)
>  		free(item->value);
>  	}
>  	free(item->counts);
> +	free(item->is_base);
>  	free(item);
>  }
>  
> @@ -3040,6 +3071,49 @@ void filter_ahead_behind(struct repository *r,
>  	free(commits);
>  }
>  
> +void filter_is_base(struct repository *r,
> +		    struct ref_format *format,
> +		    struct ref_array *array)
> +{
> +	struct commit **bases;
> +	size_t bases_nr = 0;
> +	struct ref_array_item **back_index;
> +
> +	if (!format->is_base_tips.nr || !array->nr)
> +		return;
> +
> +	CALLOC_ARRAY(back_index, array->nr);
> +	CALLOC_ARRAY(bases, array->nr);
> +
> +	for (size_t i = 0; i < array->nr; i++) {
> +		const char *name = array->items[i]->refname;
> +		struct commit *c = lookup_commit_reference_by_name(name);
> +
> +		CALLOC_ARRAY(array->items[i]->is_base, format->is_base_tips.nr);
> +
> +		if (!c)
> +			continue;

Hmph, wouldn't we want to leave array->items[i]->is_base NULL if
"name" looked up to "c" happens to be non-commit (i.e. NULL)?

> +		back_index[bases_nr] = array->items[i];
> +		bases[bases_nr] = c;
> +		bases_nr++;
> +	}


Thanks.