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

[Jonathan Tan <jonathantanmy@google.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)"?

"(and the like)" sounds good.

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

I just did the minimal change to fix the bug in the test.

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

Maybe add the following to the end of the last paragraph of the commit
message:

  (dwim_ref() is unchanged because I expect more and more code to use
  repo_dwim_ref(), and to reduce the size of the diff.)

If you prefer not to make this change locally, let me know and I'll
resend one with the updated commit message and the "(and the like)"
change above.

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

Same as above - I only did the minimal change to fix the bug.
Admittedly, if a reflog-accessing command could fail in the same way
(dangling mark), we would need to fix repo_dwim_log() and then we could
simplify substitute_branch_name() to not take the nonfatal_dangling_mark
parameter (since all dangling marks would now be nonfatal), but I
haven't looked beyond this.