Re: :/regex syntax picks stash entries over regular commits

[Jeff King <peff@peff.net>]

On Sun, Mar 31, 2024 at 04:06:35PM -0700, Junio C Hamano wrote:

> "Nude F. Ninja" <nudefninja@gmail.com> writes:
> 
> > What did you do before the bug happened? (Steps to reproduce your issue)
> > I ran git stash push, which created the stash entry "On main: dark
> > mode". Then I committed changes before noticing an oversight with the
> > previous commit. I wrote the fix and ran git commit --fixup :/dark
> 
> It is natural that there are multiple commits that match the pattern
> you give in your repository.
> 
> One trick I learned that is effective is to explicitly state where
> to start searches, e.g. "--fixup 'HEAD^{/dark mode}'", which would
> be very much in line to what --fixup wants to do.  The commit to be
> fixed up in a later rebase session by definition must be an ancestor
> of the current HEAD.

Yeah, the "traverse all commits" aspect of ":/" is well known and
confusing, and why we introduced the rev ^{/} syntax. But I still wonder
if it would be better to limit ":/" to something more sensible. Finding
"refs/stash" or "refs/notes/*" is downright confusing (stash especially
because we don't walk the reflog, so it sees only stash@{0}, and not the
others!).

It would be pretty easy to do the equivalent of "--branches --tags
--remotes":

diff --git a/object-name.c b/object-name.c
index 523af6f64f..5285903f78 100644
--- a/object-name.c
+++ b/object-name.c
@@ -2002,7 +2002,9 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
 
 			cb.repo = repo;
 			cb.list = &list;
-			refs_for_each_ref(get_main_ref_store(repo), handle_one_ref, &cb);
+			refs_for_each_ref_in(get_main_ref_store(repo), "refs/heads/", handle_one_ref, &cb);
+			refs_for_each_ref_in(get_main_ref_store(repo), "refs/tags/", handle_one_ref, &cb);
+			refs_for_each_ref_in(get_main_ref_store(repo), "refs/remotes/", handle_one_ref, &cb);
 			refs_head_ref(get_main_ref_store(repo), handle_one_ref, &cb);
 			commit_list_sort_by_date(&list);
 			return get_oid_oneline(repo, name + 2, oid, list);

Or alternatively to skip known-confusing parts of the namespace like
refs/stash.

I dunno. I have long ago written off :/ as useless, so maybe trying to
make it slightly less confusing is a fool's errand. Maybe we'd be better
off putting a note in its documentation that rev^{/} is more likely to
do what you want.

-Peff