Re: :/regex syntax picks stash entries over regular commits
- From
Jeff King <peff@peff.net>
- Date
- Apr 1, 2024, 02:44 UTC
- Message-ID
- <20240401024411.GA2639525@coredump.intra.peff.net>
- In-Reply-To
- <xmqqy19ydmro.fsf@gitster.g>
On Sun, Mar 31, 2024 at 04:06:35PM -0700, Junio C Hamano wrote:
Show 15 quoted lines
> "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