Skip to content
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

fix lite: peek space skip comment #1218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

YydsNone
Copy link

I'm now using [email protected] and noticed that pattern (?x)[0-#-contrived comment!\n9]+ and pattern (?x)[0-# contrived comment!\n9]+ behave differently. I think the cause is that Parser's peek_space method doesn't skip comment correctly (not consistent with bump_space method). below is a simple comparison between (?x)[0-#-contrived comment!\n9]+, (?x)[0-#]contrived comment!\n9]+ and (?x)[0-# contrived comment!\n9]+.

#[cfg(test)]
mod tests {
    use regex_lite::Regex;

    #[test]
    fn test_regex_lite() {
        let hyphen_pattern = "(?x)[0-#-contrived comment!\n9]+";
        let right_square_bracket_pattern = "(?x)[0-#]contrived comment!\n9]+";
        let another_pattern = "(?x)[0-# contrived comment!\n9]+";

        let haystack = "0123456789";

        let hyphen_match = Regex::new(hyphen_pattern).unwrap().find(haystack).unwrap();
        let right_square_bracket_match = Regex::new(right_square_bracket_pattern)
            .unwrap()
            .find(haystack)
            .unwrap();
        let another_match = Regex::new(another_pattern).unwrap().find(haystack).unwrap();
        assert_eq!(hyphen_match, right_square_bracket_match);
        assert_eq!(hyphen_match, another_match);
    }
}
test tests::test_regex_lite ... FAILED

failures:

---- tests::test_regex_lite stdout ----
thread 'tests::test_regex_lite' panicked at src/main.rs:24:9:
assertion `left == right` failed
  left: Match { start: 0, end: 1, string: "0" }
 right: Match { start: 0, end: 10, string: "0123456789" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@BurntSushi
Copy link
Member

Can you compare this with how regex behaves please?

@YydsNone
Copy link
Author

Can you compare this with how regex behaves please?

#[cfg(test)]
mod tests {
    use regex::Regex;

    #[test]
    fn test_regex_lite() {
        let hyphen_pattern = "(?x)[0-#-contrived comment!\n9]+";
        let right_square_bracket_pattern = "(?x)[0-#]contrived comment!\n9]+";
        let another_pattern = "(?x)[0-# contrived comment!\n9]+";

        let haystack = "0123456789";

        let hyphen_match = Regex::new(hyphen_pattern).unwrap().find(haystack).unwrap();
        let right_square_bracket_match = Regex::new(right_square_bracket_pattern)
            .unwrap()
            .find(haystack)
            .unwrap();
        let another_match = Regex::new(another_pattern).unwrap().find(haystack).unwrap();
        assert_eq!(hyphen_match, right_square_bracket_match);
        assert_eq!(hyphen_match, another_match);
    }
}
running 1 test
test tests::test_regex_lite ... FAILED

failures:

---- tests::test_regex_lite stdout ----
thread 'tests::test_regex_lite' panicked at src/main.rs:24:9:
assertion `left == right` failed
  left: Match { start: 0, end: 1, string: "0" }
 right: Match { start: 0, end: 10, string: "0123456789" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@YydsNone
Copy link
Author

test codes are basically the same, it seems that the behavior of regex and regex-lite is consistent

self.pattern()[start..].chars().next()
let char_offset = self.pattern[start..].chars().position(|ch| {
let ori_in_command = in_comment;
// detailed representation: (in_comment && ch != '\n') || (!in_comment || ch == '#')
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(!in_commit || ch == '#') -> (!in_commit && ch == '#')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants