Re: [PATCH] sha1-name.c: for ":/", find detached HEAD commits

From: William Chargin <wchargin@gmail.com>
Date: Jul 11, 2018, 06:18 UTC
Message-ID: <CAFW+GMAP6hwU1frm-FUVqx4pokpSQajWz7N6mGtMC-EO+01m2Q@mail.gmail.com>
In-Reply-To: <xmqqwou26hcv.fsf@gitster-ct.c.googlers.com>

> we care about reachability only from the detached HEAD here, so this
> must _not_ use test_commit, which creates an extra tag.

Right. I can add a comment to that effect.

> Please avoid unnecessary reflowing of earlier lines of the paragrpah
> when the only change is to insert "or from HEAD" in the middle of its
> fourth line.

Sure: I'll make that change. (My intent was to incrementally clean up an
area already under change, but I'm happy to instead make only the
minimal change.)

> Also, I am not sure if "or from HEAD" is even needed when we say
> "from ANY ref" already, as we count things like HEAD as part of the
> ref namespace.

My two cents: with the docs as is, I wasn't sure whether HEAD was intended to count as a ref for this purpose. The gitglossary man page defines a ref as a "name that begins with refs/" (seemingly excluding HEAD), though it later says that HEAD is a "special-purpose ref". In my opinion, the change adds clarity without any particular downside---but I'm happy to revert it if you'd prefer. I'd also be happy to change the wording to something like "any ref, including HEAD" if we want to emphasize that HEAD really is a ref.

> We are interested in seeing that ":/string" is understood as a valid
> object name, and this is not limited to "log" at all.

Indeed. I was a bit surprised for the same reason (I expected the tests to be using `rev-parse`), but I agree that it's probably best to keep the existing structure to minimize churn.

---

After reaching consensus on the change to the docs, should I send in a [PATCH v2] In-Reply-To this thread? Peff, should I add your Signed-off-by to the commit message, or is that not how things are done?

Best, WC