-
Notifications
You must be signed in to change notification settings - Fork 319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
config: introduce default at = @
revset alias
#4432
base: main
Are you sure you want to change the base?
Conversation
There's no changelog or docs yet but I'll get to it shortly assuming nobody has strong feelings on this. |
I am honored!
"z"? 🤔 |
When using Windows, `@` is a bit of an annoyance to type in the terminal due to Windows Terminal escape rules, so you actually have to write it with a backtick in front. It's annoying to remember this and not possible to configure a different behavior, as far as I can see. However, there is a smart trick you can do to make this a lot easier: just alias `@` to the name `at` and be happy. Because: - It's short: sweet and easy to read, and requires no backticks. - It's unambiguous: `a` is from commit alphabet, but `t` is from the change alphabet - It can be typed without a huge loss in efficiency (and one hand, assuming QWERTY). I believe it was Stephen Jennings who came up with this trick first in Discord, and I have it in my aliases, but I run into it every time I try to use JJ on a fresh Windows VM to test stuff. Let's make life a little easier for everyone and just ship this by default. Signed-off-by: Austin Seipp <[email protected]>
41aef9f
to
d730e6f
Compare
@@ -22,3 +22,6 @@ latest( | |||
'immutable_heads()' = 'builtin_immutable_heads()' | |||
'immutable()' = '::(immutable_heads() | root())' | |||
'mutable()' = '~immutable()' | |||
|
|||
# NOTE: for Windows users, who don't want to type `@ all the time | |||
'at' = '@' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Symbol aliases overrides tags and branches, so builtin aliases have to be all functions. And at()
wouldn't be easy to type.
Maybe add doc example instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, that's unfortunately a pretty big hole in the side of the ship, so to speak. A doc example is at least in order though.
I also feel like this is something we should should have tests for i.e. iff there exists any keys in [revset-aliases]
in any of the builtin configs, and they are not functions, then panic — since I wouldn't have thought of this without your suggestion. So maybe I should fix that, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a more verbose way to specify a branch/tag specifically? I feel like at
will not be a common name for those, so as long as there’s an override it might be okay.
Also, it should maybe say “PowerShell” rather than “Windows” here, as… technically you can use it on other OSes. Technically. (And there was some confusion about cmd.exe
not requiring it on the Discord.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"at"
or tags(exact:at)
works, but less special case is better. It would be weird if trunk
was a builtin alias and not pointing to the trunk
branch. The at
alias looks similar.
I also think that, to work around PowerShell, it's better to find another symbol (e.g. .
) to replace @
, instead of adding a builtin alias which only PowerShell users would practically use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One reason I didn't pick .
for the working copy was that it seemed like it would be confusing to former Mercurial users (.
is the parent of the working copy there). Also, the @
symbol works well in the graph log output. I'm not sure .
would work as well there. It might be hard to notice, and it might be hard to realize the correspondence with the revset symbol. Were you thinking of shipping .
as an alias or to eventually have it replace @
?
Shipping at
by default on Windows might be okay if we think only (a subset of) Windows users would use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do people think about Yuya's proposal to simply document the alias? Seems fine to at least start with that and see how many reports we get after that?
Btw, if the @
symbol is annoying for PowerShell users, then our use of it in remote bookmarks and workspace identifiers will also be a problem, although a much less frequent one. I suppose one possible alternative syntax that works particularly well for remote bookmarks is origin/main
, but I'm not sure I like /
for the working copy and /other
for the working copy in workspace other
as much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s okay in those contexts because it isn’t the first character in a token. main@origin
isn’t a problem because PowerShell doesn’t interpret that as the start of an array since it’s after a word character.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. @other
(which refers to the working-copy in workspace other
) is still a problem then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, so default@
, ..@
, etc. work. The workspace id syntax is <id>@
, btw.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do people think about Yuya's proposal to simply document the alias?
We actually already document it, sort of, in https://martinvonz.github.io/jj/latest/windows/#typing-in-powershell. That recommends HEAD
, which is not as good as at
IMO.
Should we make rename it and make it more prominent? I think so. It'd make sense to explain it in the main config doc and then link to it again from the Windows doc. Update: Even on non-Windows, it's nice to not have to reach for the Shift key sometimes.
Separately (and not blocking the more obvious improvements), should we explain the "at"
syntax for a branch/tag next to where we describe revset aliases?
Separately again, I still wonder about the larger issue. While we reduce the problem of overlaps with branch names if we don't define at
ourselves, it'd be nice to have a solution that also works for users who configured it themselves (and maybe forget). But we can ponder and/or discuss this question long after we fix the easy-to-fix things.
In #4432 I submitted a patch to ship a symbol alias in the default configuration. However, Yuya quickly pointed out that this isn't acceptable because symbols override all branch and trunk names, meaning that the name `at` would have at best been taken away from users, and at worst been very confusing for them down the road had they ever tried to use it. I should have thought of this myself (I encountered it long ago when first using Jujutsu, actually) but didn't. Such is life. But this didn't cause any tests to fail, as it was not immediately clear this would impede anything or cause downstream behavioral changes; the problems it adds are actually latent and it's very possible someone will want to make the same mistake in the future. And Yuya might not be able to stop them (me) in time; requiring him to go out of his way and then handle the fallout. I'm not a fan of this. Instead, let's patch this latent hole for good, or until we decide otherwise, by applying the Beyonce Rule: "if you liked that behavior, then you shoulda put a test on it." Signed-off-by: Austin Seipp <[email protected]>
In #4432 I submitted a patch to ship a symbol alias in the default configuration. However, Yuya quickly pointed out that this isn't acceptable because symbols override all branch and tag names, meaning that the name `at` would have at best been taken away from users, and at worst been very confusing for them down the road had they ever tried to use it. I should have thought of this myself (I encountered it long ago when first using Jujutsu, actually) but didn't. Such is life. But this didn't cause any tests to fail, as it was not immediately clear this would impede anything or cause downstream behavioral changes; the problems it adds are actually latent and it's very possible someone will want to make the same mistake in the future. And Yuya might not be able to stop them (me) in time; requiring him to go out of his way and then handle the fallout. I'm not a fan of this. Instead, let's patch this latent hole for good, or until we decide otherwise, by applying the Beyonce Rule: "if you liked that behavior, then you shoulda put a test on it." Signed-off-by: Austin Seipp <[email protected]>
When using Windows,
@
is a bit of an annoyance to type in the terminal due to Windows Terminal escape rules, so you actually have to write it with a backtick in front. It's annoying to remember this and not possible to configure a different behavior, as far as I can see.However, there is a smart trick you can do to make this a lot easier: just alias
@
to the nameat
and be happy. Because:a
is from commit alphabet, butt
is from the change alphabetI believe it was Stephen Jennings who came up with this trick first in Discord, and I have it in my aliases, but I run into it every time I try to use JJ on a fresh Windows VM to test stuff. Let's make life a little easier for everyone and just ship this by default.
Checklist
If applicable:
CHANGELOG.md