| Message ID | 20201122164641.2091160-1-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | refspec: make @ a valid refspec | expand |
On Sun, Nov 22, 2020 at 10:46:41AM -0600, Felipe Contreras wrote: > Since commit 9ba89f484e git learned how to push to a remote branch using > the source @, for example: > > git push origin @:master > > However, if the right-hand side is missing, the push fails: > > git push origin @ Yeah, I think this probably makes sense. I find the "@:master" refspec a bit weird, but it is a natural consequence of parsing "@" on the LHS as an arbitrary object to lookup (not a ref). I did find the implementation a little curious: > - item->src = xstrndup(lhs, llen); > + if (llen == 1 && *lhs == '@') > + item->src = xstrdup("HEAD"); > + else > + item->src = xstrndup(lhs, llen); We already know we are parsing the LHS, so why must we expand a bare @, whereas we do not in "@:master". The answer is that the "@" is not expanded until later, and parse_refspec() gets unhappy that "@" is also the implicit value for the RHS. This also means that: git push origin @:foo will now work when "foo" doesn't exist on the remote (if we find no match on the remote, we guess how it should be fully qualified; one of our heuristics involves seeing that HEAD points to a branch, and therefore the other side should be a branch, too). > It is obvious what is the desired behavior, and allowing the push makes > things more consistent. This same code is used by fetch, as well. There "git fetch origin @:master" does not work at all now. This would make that work (to fetch the remote HEAD into master), along with "git fetch origin @" (into FETCH_HEAD only). Both seem sensible, and I cannot think of any other reasonable meaning for them. > --- > refspec.c | 5 ++++- > t/t5511-refspec.sh | 2 ++ > t/t5516-fetch-push.sh | 9 +++++++++ > 3 files changed, 15 insertions(+), 1 deletion(-) Would we want to cover the extra cases I mentioned above (@:foo, and the two fetches), as well? I wondered if there was a good place to mention this in the refspec documentation, but it may just be an obvious fallout of the "@ is a shortcut for HEAD" definition in gitrevisions(7). The only change is that we're resolving the shortcut sooner so that more code can take advantage of it. -Peff
Jeff King <peff@peff.net> writes: > I wondered if there was a good place to mention this in the refspec > documentation, but it may just be an obvious fallout of the "@ is a > shortcut for HEAD" definition in gitrevisions(7). The only change is > that we're resolving the shortcut sooner so that more code can take > advantage of it. I too find that "@ is a shortcut for HEAD" looks ugly both at the UI level and at the implementation level [*1*], but as long as we have it, it is good to try to be consistent and allow "@" everywhere where one would write "HEAD" in places where it is syntacitically infeasible---at least we would be consistently ugly that way ;-). The title of the change may want to be clarified to help readers of "git shortlog". It's not like it is only changing "@" and no other variants valid refspec, but it forces readers to make sure that the author did not forget to deal with ":@", "src:@", "@:dst", etc. "make @ a synonym to HEAD in refspec" or something along the line, perhaps. [Footnote] *1* I shouldn't complain too much, as my own invention to silently turn "-" given from the command line into "@{-1}" is ugly at the code level the same way, even though it may be better at the UI level.
On Tue, Nov 24, 2020 at 2:01 PM Junio C Hamano <gitster@pobox.com> wrote: > > Jeff King <peff@peff.net> writes: > > > I wondered if there was a good place to mention this in the refspec > > documentation, but it may just be an obvious fallout of the "@ is a > > shortcut for HEAD" definition in gitrevisions(7). The only change is > > that we're resolving the shortcut sooner so that more code can take > > advantage of it. > > I too find that "@ is a shortcut for HEAD" looks ugly both at the UI > level and at the implementation level [*1*], but as long as we have > it, it is good to try to be consistent and allow "@" everywhere > where one would write "HEAD" in places where it is syntacitically > infeasible---at least we would be consistently ugly that way ;-). Beauty is in the eye of the beholder. I find HEAD to be an eyesore. > The title of the change may want to be clarified to help readers of > "git shortlog". It's not like it is only changing "@" and no other > variants valid refspec, but it forces readers to make sure that the > author did not forget to deal with ":@", "src:@", "@:dst", etc. > "make @ a synonym to HEAD in refspec" or something along the line, > perhaps. Right. I didn't notice it changed the semantics of those. Given that, your suggested title makes more sense. Cheers.
Felipe Contreras <felipe.contreras@gmail.com> writes: >> I too find that "@ is a shortcut for HEAD" looks ugly both at the UI >> level and at the implementation level [*1*], but as long as we have >> it, it is good to try to be consistent and allow "@" everywhere >> where one would write "HEAD" in places where it is syntacitically >> infeasible---at least we would be consistently ugly that way ;-). > > Beauty is in the eye of the beholder. Using "@" is rather "illogical" than "ugly", and at that point it is not so subjective. @-mark leads a suffix that applies some "magic" to what comes before it (e.g. next@{1}, maint@{2.weeks.ago}, and master@{-1}). Making @ a synonym for HEAD means '@' sometimes means a ref and most of the time means the introducer of magic that applies to a ref. Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th previous commit the current branch pointed at, so a mnemonic for the end user to remember the distinction between the two is that a bare "@" is different from HEAD, which is a total opposite X-<. This is all water under the bridge, though ;-) > Given that, your suggested title makes more sense. Sounds good.
On Tue, Nov 24, 2020 at 2:45 PM Junio C Hamano <gitster@pobox.com> wrote: > > Felipe Contreras <felipe.contreras@gmail.com> writes: > > >> I too find that "@ is a shortcut for HEAD" looks ugly both at the UI > >> level and at the implementation level [*1*], but as long as we have > >> it, it is good to try to be consistent and allow "@" everywhere > >> where one would write "HEAD" in places where it is syntacitically > >> infeasible---at least we would be consistently ugly that way ;-). > > > > Beauty is in the eye of the beholder. > > Using "@" is rather "illogical" than "ugly", and at that point it is > not so subjective. > > @-mark leads a suffix that applies some "magic" to what comes before > it (e.g. next@{1}, maint@{2.weeks.ago}, and master@{-1}). Making @ > a synonym for HEAD means '@' sometimes means a ref and most of the > time means the introducer of magic that applies to a ref. > > Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th > previous commit the current branch pointed at, so a mnemonic for the > end user to remember the distinction between the two is that a bare > "@" is different from HEAD, which is a total opposite X-<. > However, @{0} *does* refer to what is currently checked out, which would be head.. So in a sense @ meaning "the current branch" and applying @{0} would always be HEAD, no? I think it sort of works, but yea it is a bit messy. Thanks, Jake
Jacob Keller <jacob.keller@gmail.com> writes: >> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th >> previous commit the current branch pointed at, so a mnemonic for the >> end user to remember the distinction between the two is that a bare >> "@" is different from HEAD, which is a total opposite X-<. >> > > However, @{0} *does* refer to what is currently checked out, which > would be head.. So in a sense @ meaning "the current branch" and > applying @{0} would always be HEAD, no? Not really. It happens to hold true for @{0}, because by definition you couldn't have been on a different branch than the current one when you made the topmost commit on the current branch. For @{1} and higher, it is always "where was the current branch at N commits ago?" which is different from "where was the HEAD at N commits ago?", unless you always use a single branch and never switch away.
On Tue, Nov 24, 2020 at 3:14 PM Junio C Hamano <gitster@pobox.com> wrote: > > Jacob Keller <jacob.keller@gmail.com> writes: > > >> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th > >> previous commit the current branch pointed at, so a mnemonic for the > >> end user to remember the distinction between the two is that a bare > >> "@" is different from HEAD, which is a total opposite X-<. > >> > > > > However, @{0} *does* refer to what is currently checked out, which > > would be head.. So in a sense @ meaning "the current branch" and > > applying @{0} would always be HEAD, no? > > Not really. > > It happens to hold true for @{0}, because by definition you couldn't > have been on a different branch than the current one when you made > the topmost commit on the current branch. For @{1} and higher, it > is always "where was the current branch at N commits ago?" which is > different from "where was the HEAD at N commits ago?", unless you > always use a single branch and never switch away. > Right, once you add anything greater than zero it breaks down.. but think about it a little differently: "@{N}" is sort of eliding the branch name, which means we use the current branch. "branch@" (if it were valid syntax) would be eliding the number which means "the most recent version of branch". Thus, eliding both and using just "@" would mean "the most recent version of the current branch", which cannot be anything other than HEAD. Of course I agree that "@ == HEAD" can't be used to go *backwards* through that logic at all. But if you're moving forwards through it, then "@" on its own can make sense as HEAD, but only as an implication of "the most recent version of the current branch can't be anything else" Thanks, Jake
On Tue, Nov 24, 2020 at 4:45 PM Junio C Hamano <gitster@pobox.com> wrote: > > Felipe Contreras <felipe.contreras@gmail.com> writes: > > >> I too find that "@ is a shortcut for HEAD" looks ugly both at the UI > >> level and at the implementation level [*1*], but as long as we have > >> it, it is good to try to be consistent and allow "@" everywhere > >> where one would write "HEAD" in places where it is syntacitically > >> infeasible---at least we would be consistently ugly that way ;-). > > > > Beauty is in the eye of the beholder. > > Using "@" is rather "illogical" than "ugly", and at that point it is > not so subjective. I disagree. > @-mark leads a suffix that applies some "magic" to what comes before > it (e.g. next@{1}, maint@{2.weeks.ago}, and master@{-1}). Making @ > a synonym for HEAD means '@' sometimes means a ref and most of the > time means the introducer of magic that applies to a ref. All this was discussed back then [1]. In my mind "@" means the same as "@{0}", and "@{0}~0^0". So I don't see any inconsistency. Your mind may be different. > Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th > previous commit the current branch pointed at, so a mnemonic for the > end user to remember the distinction between the two is that a bare > "@" is different from HEAD, which is a total opposite X-<. Once again; each user is different. Personally I never use these shortcuts precisely because I'm not entirely sure what I want to tell git. I just use "git reflog" and manually pick the commit id I want. I suspect most users don't use this notation either, so even if there's an inconsistency (which I'm not entirely sure the @ shortcut has anything to do with it), it would affect few users. > This is all water under the bridge, though ;-) Indeed. And I take it we can agree that it's better to have instructions like: git push -u origin @ Rather than: git push -u origin master Regardless of what some users think of "@", it's less contentious than "master". Cheers. [1] https://lore.kernel.org/git/1367264106-2351-1-git-send-email-felipe.contreras@gmail.com/
On Tue, Nov 24, 2020 at 5:14 PM Junio C Hamano <gitster@pobox.com> wrote: > > Jacob Keller <jacob.keller@gmail.com> writes: > > However, @{0} *does* refer to what is currently checked out, which > > would be head.. So in a sense @ meaning "the current branch" and > > applying @{0} would always be HEAD, no? > > Not really. > > It happens to hold true for @{0}, because by definition you couldn't > have been on a different branch than the current one when you made > the topmost commit on the current branch. For @{1} and higher, it > is always "where was the current branch at N commits ago?" which is > different from "where was the HEAD at N commits ago?", unless you > always use a single branch and never switch away. But @{0} is always the same as HEAD@{0}, which is always the same as @.
On Tue, Nov 24, 2020 at 5:47 PM Jacob Keller <jacob.keller@gmail.com> wrote: > > On Tue, Nov 24, 2020 at 3:14 PM Junio C Hamano <gitster@pobox.com> wrote: > > > > Jacob Keller <jacob.keller@gmail.com> writes: > > > > >> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th > > >> previous commit the current branch pointed at, so a mnemonic for the > > >> end user to remember the distinction between the two is that a bare > > >> "@" is different from HEAD, which is a total opposite X-<. > > >> > > > > > > However, @{0} *does* refer to what is currently checked out, which > > > would be head.. So in a sense @ meaning "the current branch" and > > > applying @{0} would always be HEAD, no? > > > > Not really. > > > > It happens to hold true for @{0}, because by definition you couldn't > > have been on a different branch than the current one when you made > > the topmost commit on the current branch. For @{1} and higher, it > > is always "where was the current branch at N commits ago?" which is > > different from "where was the HEAD at N commits ago?", unless you > > always use a single branch and never switch away. > > Right, once you add anything greater than zero it breaks down.. but > think about it a little differently: "@{N}" is sort of eliding the > branch name, which means we use the current branch. "branch@" (if it > were valid syntax) would be eliding the number which means "the most > recent version of branch". Thus, eliding both and using just "@" would > mean "the most recent version of the current branch", which cannot be > anything other than HEAD. Yes, but to me HEAD is supposed to mean "the current branch", so "branch@{1}" and "HEAD@{1}" should be the same, and they are not. So to me they are the other way around. 1. branch@{1}, HEAD@{1}, @@{1}: current branch 1 commit ago 2. @{1}: 1 commit ago in general Since it's the opposite of what I would expect, I simply don't use these notations.
Jacob Keller <jacob.keller@gmail.com> writes: > Of course I agree that "@ == HEAD" can't be used to go *backwards* > through that logic at all. But if you're moving forwards through it, > then "@" on its own can make sense as HEAD, but only as an implication > of "the most recent version of the current branch can't be anything > else" I'd rather put the lame excuses behind and accept that we ended up with an ugly and illogical synonym that ;-) And making it consistently available in places where HEAD is accepted is a good thing. Depending on the keyboard, it may be easy to type, too.
Hello, On Tue, Nov 24, 2020 at 6:09 PM Felipe Contreras <felipe.contreras@gmail.com> wrote: > On Tue, Nov 24, 2020 at 4:45 PM Junio C Hamano <gitster@pobox.com> wrote: > > Felipe Contreras <felipe.contreras@gmail.com> writes: > > > Beauty is in the eye of the beholder. > > > > Using "@" is rather "illogical" than "ugly", and at that point it is > > not so subjective. > > I disagree. This is not an attempt to reignite the discussion, but just something that might be of interest. I created a poll [1] in reddit's r/git: Do you know @ is a shortcut for HEAD? 403 people gave a useful response. Of those; 74% didn't know such a shortcut existed, 80% liked this shortcut. But more importantly: 59% did like the shortcut, but didn't know it existed. That is: of the people that liked the shortcut; 74% didn't know it existed. This tells us something: putting personal preferences aside; the Git project doesn't seem to be doing a great job of advertising its features. Features a good chunk--if not the overwhelming majority--of users might like. Other things of interest in the comments: one user didn't even know what HEAD was, another used "head" (lowercase) (macOS user), and lastly; the second most voted comment did take a shot at Git's known unintuitiveness. That's it. Just putting it here. Cheers. [1] https://www.reddit.com/r/git/comments/k15cqm/do_you_know_is_a_shortcut_for_head/
diff --git a/refspec.c b/refspec.c index c49347c2d7..adbfb3283a 100644 --- a/refspec.c +++ b/refspec.c @@ -71,7 +71,10 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet } item->pattern = is_glob; - item->src = xstrndup(lhs, llen); + if (llen == 1 && *lhs == '@') + item->src = xstrdup("HEAD"); + else + item->src = xstrndup(lhs, llen); flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0); if (item->negative) { diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh index f541f30bc2..f808649de4 100755 --- a/t/t5511-refspec.sh +++ b/t/t5511-refspec.sh @@ -58,6 +58,8 @@ test_refspec fetch 'HEAD~4:refs/remotes/frotz/new' invalid test_refspec push 'HEAD' test_refspec fetch 'HEAD' +test_refspec push '@' +test_refspec fetch '@' test_refspec push 'refs/heads/ nitfol' invalid test_refspec fetch 'refs/heads/ nitfol' invalid diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index d11382f769..12c0c7c06b 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -501,6 +501,15 @@ test_expect_success 'push with config remote.*.push = HEAD' ' check_push_result testrepo $the_first_commit heads/local ' +test_expect_success 'push with @' ' + + mk_test testrepo heads/master && + git checkout master && + git push testrepo @ && + check_push_result testrepo $the_commit heads/master + +' + test_expect_success 'push with remote.pushdefault' ' mk_test up_repo heads/master && mk_test down_repo heads/master &&
Since commit 9ba89f484e git learned how to push to a remote branch using the source @, for example: git push origin @:master However, if the right-hand side is missing, the push fails: git push origin @ It is obvious what is the desired behavior, and allowing the push makes things more consistent. Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> --- refspec.c | 5 ++++- t/t5511-refspec.sh | 2 ++ t/t5516-fetch-push.sh | 9 +++++++++ 3 files changed, 15 insertions(+), 1 deletion(-)