Skip to content

Fix two LSH issues - #901

Merged
lhecker merged 1 commit into
mainfrom
dev/lhecker/lsh-fixup
Jul 28, 2026
Merged

Fix two LSH issues#901
lhecker merged 1 commit into
mainfrom
dev/lhecker/lsh-fixup

Conversation

@lhecker

@lhecker lhecker commented Jul 28, 2026

Copy link
Copy Markdown
Member
  • Fixes fast-skip computation for regexes that start
    with an optional alternation like [ab]?c
  • Adds support for specifying non-ASCII regex atoms
    via \xNN escape sequences

@lhecker
lhecker requested a review from DHowett July 28, 2026 16:20
't' => Ok(Atom::Char('\t')),
'x' => {
if let Some(byte) =
self.rest().get(..2).and_then(|hex| u8::from_str_radix(hex, 16).ok())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i do not comprehend how consuming only 2 characters after x will allow for something that is > 0xff in Atom::Char above

@lhecker lhecker Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Atom::Char can contain any codepoints! For instance, a regex like: [😄-🙂]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes but..

Adds support for specifying non-ASCII regex atoms
via \xNN escape sequences

i mean, technically this is true because 0x80-0xff is not ASCII
but it is surely not true in the spirit of it ;P

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, sorta. I need this to implement partial UTF8 support in one particular place by writing [^'][\x80-\xBF]*

@lhecker
lhecker merged commit b05f727 into main Jul 28, 2026
9 checks passed
@lhecker
lhecker deleted the dev/lhecker/lsh-fixup branch July 28, 2026 16:39
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