`jj git push --named` fails to handle `--` in bookmark names

[indygreg]

This is very similar to #7593 but I'm filing against jj git push --named.

My work has an almost universal practice to use -- in names as a delimiter. This is causing a bit of confusion with jj adopters since the current error message when double dash is encountered isn't very actionable. This is especially true in the jj git push --named ... case. e.g.:

$ jj git push --named 'foo--x=@'
Error: Could not parse 'foo--x' as a bookmark name
Caused by:
1: Failed to parse bookmark name: Syntax error
2:  --> 1:4
  |
1 | foo--x
  |    ^---
  |
  = expected <EOI>
Hint: For example, `--named myfeature=@` is valid syntax

This error seems very odd to me because I would expect the --named parser to split on = and treat the left side as an identifier. But I think it is somehow treating the LHS as a revset, yielding the syntax error?

In all cases (including #7593), I find the error message in these cases a bit confusing. It says Syntax error but isn't saying which syntax. (I can infer it is a revset but this may not be obvious to novice jj users.) Perhaps the hint could be improved to mention revset or suggest quoting techniques if the expression is likely not a revset (lacks parentheses, spaces, etc)?

[yuja]

LHS is a bookmark name in revset syntax, so you'll need inner quotes.

I agree that the error message is unclear.

[martinvonz]

What would break if we interpreted the LHS as a literal bookmark name? Is it that we want to prevent the user from accidentally creating bookmarks with names (such as the example above) that will require quoting when they're used later?

[indygreg]

What would break if we interpreted the LHS as a literal bookmark name?

I could potentially see a power user wanting to combine revsets and templates to dynamically derive the name here. Kinda like what templates.git_push_bookmark does. But that's templating, not revset syntax. (Or are those the same things?) I'm admittedly new to jj, but I can't think of an obvious use case for executing revsets in the LHS of jj git push --named.

(Oh hi, Martin and Yuya - it's been a while!)

[yuja]

What would break if we interpreted the LHS as a literal bookmark name? Is it that we want to prevent the user from accidentally creating bookmarks with names (such as the example above) that will require quoting when they're used later?

Yes, that's the reason why we've made jj bookmark subcommands reject revset-like bookmark names by default. --named should be the same.

For the original problem, we can also relax the parser to accept repeated middle - as symbol characters.

[martinvonz]

If we allow internal -- (and ++? +-? -.?) without quoting, then we will close the door to using such sequences as operators in the future. I think that's probably fine because we haven't had much need for new operators so far. I still wanted to mention it in case others see a problem with it.

[PhilipMetzger]

This should be fixed since #8804 landed but GH didn't close the issue again.