Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
revset: allow consecutive '-' and '+' in bookmark names
This fixes issue #8673 where  would fail
because the revset parser did not accept double dashes in bookmark names.

The grammar has been updated to allow:
- Consecutive '-' and '+' characters (e.g., foo--bar, foo++bar, foo+-bar)
- Mixed patterns with '.' (e.g., foo.-bar, foo.+bar)

The '..' sequence is still NOT allowed as part of an identifier because
it's the range operator. So 'foo..bar' is still parsed as a range from
'foo' to 'bar'.

Added tests for:
- Parser-level identifier tests for double/triple dashes
- CLI-level test for git push --named with double dashes

Closes #8673

Co-authored-by: 2020.raj.pabnani <2020.raj.pabnani@ves.ac.in>
  • Loading branch information
cursoragent and rajpabnani committed Jan 24, 2026
commit 514b4425f54870693d4768ba1adae7b484407af6
41 changes: 41 additions & 0 deletions cli/tests/test_git_push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,47 @@ fn test_git_push_changes_with_name_untracked_or_forgotten() {
");
}

/// Test that bookmark names with double dashes (--) are allowed (issue #8673)
#[test]
fn test_git_push_named_with_double_dashes() {
let test_env = TestEnvironment::default();
set_up(&test_env);
let work_dir = test_env.work_dir("local");
work_dir.run_jj(["describe", "-m", "test"]).success();
work_dir.write_file("file", "contents");

// Bookmark names with double dashes should be allowed
let output = work_dir.run_jj(["git", "push", "--named", "foo--bar=@"]);
insta::assert_snapshot!(output, @"
------- stderr -------
Changes to push to origin:
Add bookmark foo--bar to 398723676afc
[EOF]
");

// Also test triple dashes
work_dir.run_jj(["new", "-m", "test2"]).success();
work_dir.write_file("file", "modified");
let output = work_dir.run_jj(["git", "push", "--named", "feature---test=@"]);
insta::assert_snapshot!(output, @"
------- stderr -------
Changes to push to origin:
Add bookmark feature---test to 9dd74023fad1
[EOF]
");

// Mixed separators should also work
work_dir.run_jj(["new", "-m", "test3"]).success();
work_dir.write_file("file", "modified2");
let output = work_dir.run_jj(["git", "push", "--named", "feature.+test--x=@"]);
insta::assert_snapshot!(output, @"
------- stderr -------
Changes to push to origin:
Add bookmark feature.+test--x to bdcfc29b4b29
[EOF]
");
}

#[test]
fn test_git_push_revisions() {
let test_env = TestEnvironment::default();
Expand Down
11 changes: 8 additions & 3 deletions lib/src/revset.pest
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,21 @@
whitespace = _{ " " | "\t" | "\r" | "\n" | "\x0c" }

// XID_CONTINUE: https://www.unicode.org/reports/tr31/#Default_Identifier_Syntax
// +, -, .: often included in tag/bookmark name or version number
// +, -, .: often included in tag/bookmark name or version number (consecutive allowed)
// /: sometimes used as a tag/bookmark namespace separator
identifier_part = @{ (XID_CONTINUE | "_" | "/")+ }
// Separator between identifier parts: consecutive "-" and "+" allowed, but "." cannot
// be followed by another ".". This ensures ".." is parsed as a range operator.
// Allowed: foo--bar, foo++bar, foo+-bar, foo.bar, foo.-bar, foo.+bar
// Not allowed as single identifier: foo..bar (parsed as range)
identifier = @{
identifier_part ~ (("." | "-" | "+") ~ identifier_part)*
identifier_part ~ ((("-" | "+")+ | "." ~ ("-" | "+")*) ~ identifier_part)*
}
// TODO: remove "/", ".", "+" for consistency with fileset?
strict_identifier_part = @{ (ASCII_ALPHANUMERIC | "_" | "/")+ }
// Same rules as identifier for separator handling
strict_identifier = @{
strict_identifier_part ~ (("." | "-" | "+") ~ strict_identifier_part)*
strict_identifier_part ~ ((("-" | "+")+ | "." ~ ("-" | "+")*) ~ strict_identifier_part)*
}

symbol = _{
Expand Down
17 changes: 13 additions & 4 deletions lib/src/revset_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1213,18 +1213,27 @@ mod tests {
parse_into_kind("foo."),
Err(RevsetParseErrorKind::SyntaxError)
);
// Multiple '.', '-', '+' are not allowed
// Multiple consecutive '.', '-', '+' separators are allowed
assert_eq!(
parse_into_kind("foo.+bar"),
Err(RevsetParseErrorKind::SyntaxError)
Ok(ExpressionKind::Identifier("foo.+bar"))
);
assert_eq!(
parse_into_kind("foo--bar"),
Err(RevsetParseErrorKind::SyntaxError)
Ok(ExpressionKind::Identifier("foo--bar"))
);
assert_eq!(
parse_into_kind("foo+-bar"),
Err(RevsetParseErrorKind::SyntaxError)
Ok(ExpressionKind::Identifier("foo+-bar"))
);
// Triple dashes, mixed separators also work
assert_eq!(
parse_into_kind("foo---bar"),
Ok(ExpressionKind::Identifier("foo---bar"))
);
assert_eq!(
parse_into_kind("feature--DT-row--click-action"),
Ok(ExpressionKind::Identifier("feature--DT-row--click-action"))
);

// Parse a parenthesized symbol
Expand Down