Re: [PATCH v2 2/2] wt-status: tolerate dangling marks

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

Jonathan Tan <jonathantanmy@google.com> writes:

> +
> +	/*
> +	 * If ^{upstream} or ^{push} (or equivalent) is requested, and the
> +	 * branch in question does not have such a reference, return -1 instead
> +	 * of die()-ing.
> +	 */
> +	unsigned nonfatal_dangling_mark : 1;

Micronit; I would have avoided "or equivalent" as the point of
parenthetical comment was not to say these two modifiers upstream
and push (and other forms that spell differently but invokes exactly
one of these two features) are special, but to say that these two
are merely examples, and any other ^{modifiers} we have or we will
add in the future would honor this bit.  Perhaps "(and the like)"?

>  };
>  int repo_interpret_branch_name(struct repository *r,
>  			       const char *str, int len,
> diff --git a/refs.c b/refs.c
> index cf09cd039f..b6f1a2f452 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -598,10 +598,13 @@ const char *git_default_branch_name(void)
>   * to name a branch.
>   */
>  static char *substitute_branch_name(struct repository *r,
> -				    const char **string, int *len)
> +				    const char **string, int *len,
> +				    int nonfatal_dangling_mark)
>  {
>  	struct strbuf buf = STRBUF_INIT;
> -	struct interpret_branch_name_options options = { 0 } ;
> +	struct interpret_branch_name_options options = {
> +		.nonfatal_dangling_mark = nonfatal_dangling_mark
> +	};
>  	int ret = repo_interpret_branch_name(r, *string, *len, &buf, &options);
>  
>  	if (ret == *len) {
> @@ -615,9 +618,10 @@ static char *substitute_branch_name(struct repository *r,
>  }
>  
>  int repo_dwim_ref(struct repository *r, const char *str, int len,
> -		  struct object_id *oid, char **ref)
> +		  struct object_id *oid, char **ref, int nonfatal_dangling_mark)
>  {
> -	char *last_branch = substitute_branch_name(r, &str, &len);
> +	char *last_branch = substitute_branch_name(r, &str, &len,
> +						   nonfatal_dangling_mark);
>  	int   refs_found  = expand_ref(r, str, len, oid, ref);
>  	free(last_branch);
>  	return refs_found;
> @@ -625,7 +629,7 @@ int repo_dwim_ref(struct repository *r, const char *str, int len,
>  
>  int dwim_ref(const char *str, int len, struct object_id *oid, char **ref)
>  {
> -	return repo_dwim_ref(the_repository, str, len, oid, ref);
> +	return repo_dwim_ref(the_repository, str, len, oid, ref, 0);
>  }
>  
>  int expand_ref(struct repository *repo, const char *str, int len,
> @@ -666,7 +670,7 @@ int repo_dwim_log(struct repository *r, const char *str, int len,
>  		  struct object_id *oid, char **log)
>  {
>  	struct ref_store *refs = get_main_ref_store(r);
> -	char *last_branch = substitute_branch_name(r, &str, &len);
> +	char *last_branch = substitute_branch_name(r, &str, &len, 0);
>  	const char **p;
>  	int logs_found = 0;
>  	struct strbuf path = STRBUF_INIT;

Among these callers that reach substitute_branch_name(), how were
those that can specify the new bit chosen?

For example, what is the reasoning behind making dwim_ref() unable
to ask the "do so gently" variant, while allowing repo_dwim_ref()
to?

I am NOT necessarily saying these two functions MUST be able to
access the same set of features and the only difference between them
MUST be kept to the current "repo_* variant can work on an arbitrary
repository, while the variant without repo_* would work on the
primary repository only".  As long as there is a good reason to make
their power diverge, it is OK---I just do not see why and I'd like
to know.

The same question about not allowing the gentler variant while
drimming the reflog.

Thanks.