On Mon, Mar 17, 2025 at 08:24:36PM -0400, Jeff King wrote: > On Mon, Mar 17, 2025 at 06:24:24PM -0400, Taylor Blau wrote: > > > Since 6d4c057859 (refspec: introduce struct refspec, 2018-05-16), we > > have constants called REFSPEC_FETCH and REFSPEC_PUSH. This misleadingly > > suggests that we might introduce other modes in the future. > > I don't know that I'd call it misleading. We _could_ introduce new modes > if we had new operations. But I do agree it's unlikely (even if we had > other operations like git-archive, cat-file, etc, they would probably > not have refspecs). > > So it seems like a reasonable direction to me. Fair, I think a more accurate statement might be to swap "misleadingly" for "confusingly". I'll swap that and amend the paragraph to say "[...], which is possible, but highly unlikely" > The one thing I don't like is: > > > diff --git a/builtin/pull.c b/builtin/pull.c > > index 9c4a00620a..8bbfcce729 100644 > > --- a/builtin/pull.c > > +++ b/builtin/pull.c > > @@ -738,7 +738,7 @@ static const char *get_tracking_branch(const char *remote, const char *refspec) > > const char *spec_src; > > const char *merge_branch; > > > > - refspec_item_init_or_die(&spec, refspec, REFSPEC_FETCH); > > + refspec_item_init_or_die(&spec, refspec, 1); > > The third argument here (and elsewhere) becomes much more mysterious to > the reader. Maybe not a big deal, though. Hmm. I see later on in the thread that the final patch resolves this awkwardness. I figured that readers might have a similar thought here, which is why I included "Note that this introduces some awkwardness like [...]", so perhaps there is a way to clarify that. But... > > diff --git a/git-diff-pairs b/git-diff-pairs > > Hmm.... :) ...I could forgive you for not noticing given this ugliness ;-). Thanks, Taylor
