Cursor/single bug resolution ecc9

cli/src/template_parser.rs

@@ -465,7 +465,9 @@ fn parse_term_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode> {
     let span = expr.as_span();
     let primary = match expr.as_rule() {
         Rule::string_literal => {
-            let text = STRING_LITERAL_PARSER.parse(expr.into_inner());
+            let text = STRING_LITERAL_PARSER
+                .parse(expr.into_inner())
+                .map_err(|err| TemplateParseError::expression(err.to_string(), span))?;
             ExpressionNode::new(ExpressionKind::String(text), span)
         }
         Rule::raw_string_literal => {
@@ -1122,9 +1124,15 @@ mod tests {
             Ok(ExpressionKind::String("aeiou".to_owned())),
         );
         assert_eq!(
-            parse_into_kind(r#""\xe0\xe8\xec\xf0\xf9""#),
+            parse_into_kind(r#""\xc3\xa0\xc3\xa8\xc3\xac\xc3\xb0\xc3\xb9""#),
             Ok(ExpressionKind::String("àèìðù".to_owned())),
         );
+        assert_eq!(
+            parse_into_kind(r#""\xc3""#),
+            Err(TemplateParseErrorKind::Expression(
+                "Invalid UTF-8 sequence".to_owned(),
+            )),
+        );
         assert_eq!(
             parse_into_kind(r#""\x""#),
             Err(TemplateParseErrorKind::SyntaxError),

cli/tests/test_git_push.rs

@@ -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();

docs/templates.md

@@ -458,7 +458,7 @@ A double-quoted string literal supports the following escape sequences:
 * `\n`: new line
 * `\0`: null
 * `\e`: escape (i.e., `\x1b`)
-* `\xHH`: byte with hex value `HH`
+* `\xHH`: byte with hex value `HH` (must be part of a valid UTF-8 string)
 
 Other escape sequences are not supported. Any UTF-8 characters are allowed
 inside a string literal, with two exceptions: unescaped `"`-s and uses of `\`

lib/src/dsl_util.rs

@@ -16,6 +16,7 @@
 
 use std::ascii;
 use std::collections::HashMap;
+use std::error;
 use std::fmt;
 use std::slice;
 
@@ -397,34 +398,44 @@ pub struct StringLiteralParser<R> {
     pub escape_rule: R,
 }
 
+#[expect(missing_docs)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub struct StringLiteralParseError;
+
+impl fmt::Display for StringLiteralParseError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("Invalid UTF-8 sequence")
+    }
+}
+
+impl error::Error for StringLiteralParseError {}
+
 impl<R: RuleType> StringLiteralParser<R> {
     /// Parses the given string literal `pairs` into string.
-    pub fn parse(&self, pairs: Pairs<R>) -> String {
-        let mut result = String::new();
+    pub fn parse(&self, pairs: Pairs<R>) -> Result<String, StringLiteralParseError> {
+        let mut result = Vec::new();
         for part in pairs {
             if part.as_rule() == self.content_rule {
-                result.push_str(part.as_str());
+                result.extend_from_slice(part.as_str().as_bytes());
             } else if part.as_rule() == self.escape_rule {
                 match &part.as_str()[1..] {
-                    "\"" => result.push('"'),
-                    "\\" => result.push('\\'),
-                    "t" => result.push('\t'),
-                    "r" => result.push('\r'),
-                    "n" => result.push('\n'),
-                    "0" => result.push('\0'),
-                    "e" => result.push('\x1b'),
+                    "\"" => result.push(b'"'),
+                    "\\" => result.push(b'\\'),
+                    "t" => result.push(b'\t'),
+                    "r" => result.push(b'\r'),
+                    "n" => result.push(b'\n'),
+                    "0" => result.push(b'\0'),
+                    "e" => result.push(b'\x1b'),
                     hex if hex.starts_with('x') => {
-                        result.push(char::from(
-                            u8::from_str_radix(&hex[1..], 16).expect("hex characters"),
-                        ));
+                        result.push(u8::from_str_radix(&hex[1..], 16).expect("hex characters"));
                     }
                     char => panic!("invalid escape: \\{char:?}"),
                 }
             } else {
                 panic!("unexpected part of string: {part:?}");
             }
         }
-        result
+        String::from_utf8(result).map_err(|_| StringLiteralParseError)
     }
 }
 

lib/src/fileset_parser.rs

@@ -236,14 +236,17 @@ fn parse_function_call_node(pair: Pair<Rule>) -> FilesetParseResult<FunctionCall
     })
 }
 
-fn parse_as_string_literal(pair: Pair<Rule>) -> String {
+fn parse_as_string_literal(pair: Pair<Rule>) -> FilesetParseResult<String> {
+    let span = pair.as_span();
     match pair.as_rule() {
-        Rule::identifier => pair.as_str().to_owned(),
-        Rule::string_literal => STRING_LITERAL_PARSER.parse(pair.into_inner()),
+        Rule::identifier => Ok(pair.as_str().to_owned()),
+        Rule::string_literal => STRING_LITERAL_PARSER
+            .parse(pair.into_inner())
+            .map_err(|err| FilesetParseError::expression(err.to_string(), span)),
         Rule::raw_string_literal => {
             let [content] = pair.into_inner().collect_array().unwrap();
             assert_eq!(content.as_rule(), Rule::raw_string_content);
-            content.as_str().to_owned()
+            Ok(content.as_str().to_owned())
         }
         r => panic!("unexpected string literal rule: {r:?}"),
     }
@@ -264,12 +267,12 @@ fn parse_primary_node(pair: Pair<Rule>) -> FilesetParseResult<ExpressionNode> {
             assert_eq!(lhs.as_rule(), Rule::strict_identifier);
             assert_eq!(op.as_rule(), Rule::pattern_kind_op);
             let kind = lhs.as_str();
-            let value = parse_as_string_literal(rhs);
+            let value = parse_as_string_literal(rhs)?;
             ExpressionKind::StringPattern { kind, value }
         }
         Rule::identifier => ExpressionKind::Identifier(first.as_str()),
         Rule::string_literal | Rule::raw_string_literal => {
-            ExpressionKind::String(parse_as_string_literal(first))
+            ExpressionKind::String(parse_as_string_literal(first)?)
         }
         r => panic!("unexpected primary rule: {r:?}"),
     };
@@ -519,9 +522,15 @@ mod tests {
             Ok(ExpressionKind::String("aeiou".to_owned())),
         );
         assert_eq!(
-            parse_into_kind(r#""\xe0\xe8\xec\xf0\xf9""#),
+            parse_into_kind(r#""\xc3\xa0\xc3\xa8\xc3\xac\xc3\xb0\xc3\xb9""#),
             Ok(ExpressionKind::String("àèìðù".to_owned())),
         );
+        assert_eq!(
+            parse_into_kind(r#""\xc3""#),
+            Err(FilesetParseErrorKind::Expression(
+                "Invalid UTF-8 sequence".to_owned(),
+            )),
+        );
         assert_eq!(
             parse_into_kind(r#""\x""#),
             Err(FilesetParseErrorKind::SyntaxError),

lib/src/revset.pest

@@ -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 = _{

lib/src/revset_parser.rs

@@ -636,14 +636,14 @@ fn parse_primary_node(pair: Pair<Rule>) -> Result<ExpressionNode, RevsetParseErr
             assert_eq!(lhs.as_rule(), Rule::strict_identifier);
             assert_eq!(op.as_rule(), Rule::pattern_kind_op);
             let kind = lhs.as_str();
-            let value = parse_as_string_literal(rhs);
+            let value = parse_as_string_literal(rhs)?;
             ExpressionKind::StringPattern { kind, value }
         }
         // Identifier without "@" may be substituted by aliases. Primary expression including "@"
         // is considered an indecomposable unit, and no alias substitution would be made.
         Rule::identifier if pairs.peek().is_none() => ExpressionKind::Identifier(first.as_str()),
         Rule::identifier | Rule::string_literal | Rule::raw_string_literal => {
-            let name = parse_as_string_literal(first);
+            let name = parse_as_string_literal(first)?;
             match pairs.next() {
                 None => ExpressionKind::String(name),
                 Some(op) => {
@@ -654,7 +654,7 @@ fn parse_primary_node(pair: Pair<Rule>) -> Result<ExpressionNode, RevsetParseErr
                         // infix "<name>@<remote>"
                         Some(second) => {
                             let name: RefNameBuf = name.into();
-                            let remote: RemoteNameBuf = parse_as_string_literal(second).into();
+                            let remote: RemoteNameBuf = parse_as_string_literal(second)?.into();
                             ExpressionKind::RemoteSymbol(RemoteRefSymbolBuf { name, remote })
                         }
                     }
@@ -669,14 +669,17 @@ fn parse_primary_node(pair: Pair<Rule>) -> Result<ExpressionNode, RevsetParseErr
 }
 
 /// Parses part of compound symbol to string.
-fn parse_as_string_literal(pair: Pair<Rule>) -> String {
+fn parse_as_string_literal(pair: Pair<Rule>) -> Result<String, RevsetParseError> {
+    let span = pair.as_span();
     match pair.as_rule() {
-        Rule::identifier => pair.as_str().to_owned(),
-        Rule::string_literal => STRING_LITERAL_PARSER.parse(pair.into_inner()),
+        Rule::identifier => Ok(pair.as_str().to_owned()),
+        Rule::string_literal => STRING_LITERAL_PARSER
+            .parse(pair.into_inner())
+            .map_err(|err| RevsetParseError::expression(err.to_string(), span)),
         Rule::raw_string_literal => {
             let [content] = pair.into_inner().collect_array().unwrap();
             assert_eq!(content.as_rule(), Rule::raw_string_content);
-            content.as_str().to_owned()
+            Ok(content.as_str().to_owned())
         }
         _ => {
             panic!("unexpected string literal rule: {:?}", pair.as_str());
@@ -697,7 +700,7 @@ pub fn parse_symbol(text: &str) -> Result<String, RevsetParseError> {
     let mut pairs = RevsetParser::parse(Rule::symbol_name, text)?;
     let first = pairs.next().unwrap();
     let span = first.as_span();
-    let name = parse_as_string_literal(first);
+    let name = parse_as_string_literal(first)?;
     if name.is_empty() {
         Err(RevsetParseError::expression(
             "Expected non-empty string",
@@ -1213,18 +1216,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
@@ -1275,9 +1287,15 @@ mod tests {
             Ok(ExpressionKind::String("aeiou".to_owned()))
         );
         assert_eq!(
-            parse_into_kind(r#""\xe0\xe8\xec\xf0\xf9""#),
+            parse_into_kind(r#""\xc3\xa0\xc3\xa8\xc3\xac\xc3\xb0\xc3\xb9""#),
             Ok(ExpressionKind::String("àèìðù".to_owned()))
         );
+        assert_eq!(
+            parse_into_kind(r#""\xc3""#),
+            Err(RevsetParseErrorKind::Expression(
+                "Invalid UTF-8 sequence".to_owned()
+            ))
+        );
         assert_eq!(
             parse_into_kind(r#""\x""#),
             Err(RevsetParseErrorKind::SyntaxError)