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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 29, 2020, 18:55 UTC
Message-ID: <xmqqo8mti8od.fsf@gitster.c.googlers.com>
In-Reply-To: <59b91a206d9d7bf64825cb48c747730e28b10a79.1598662525.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 7 quoted lines

> +
> +	/*
> +	 * 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)"?

Show 54 quoted lines

>  };
>  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.