Skip to content

Conversation

@timkaechele
Copy link
Contributor

This PR migrates the lexer peek helper interfaces to use hb_string_T instead of null terminated strings.

@timkaechele timkaechele force-pushed the lexer-peek-string-migration branch 3 times, most recently from ff80bda to 6218970 Compare October 15, 2025 18:54
Copy link
Owner

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request @timkaechele!

I'm struggling to see the motivation for this migration here. Is this so we more consistently use hb_string_T over char*, or is there something else I'm missing?

@timkaechele
Copy link
Contributor Author

Thanks for the pull request @timkaechele!

I'm struggling to see the motivation for this migration here. Is this so we more consistently use hb_string_T over char*, or is there something else I'm missing?

Mostly for consistency, and because I think we can later refactor the lexer_peek_for function to utilize our string helper functions.

@timkaechele timkaechele marked this pull request as ready for review October 16, 2025 17:32
@timkaechele timkaechele force-pushed the lexer-peek-string-migration branch 2 times, most recently from ededbc1 to ac8181f Compare October 16, 2025 19:10
Comment on lines +20 to +21
hb_string_T remaining_source = hb_string_slice(lexer->source, lexer->current_position + offset);
remaining_source.length = MIN(pattern.length, remaining_source.length);
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, this is elegant! 😍

Copy link
Owner

@marcoroth marcoroth left a comment

Choose a reason for hiding this comment

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

Thanks @timkaechele! 🙌🏼

@timkaechele timkaechele force-pushed the lexer-peek-string-migration branch from dcc7e9e to 004212f Compare October 17, 2025 16:46
@marcoroth marcoroth changed the title C: Use hb_string_T in lexer peek helper C: Use hb_string_T in lexer_peek_helpers.c Oct 17, 2025
@marcoroth marcoroth merged commit 1371eb3 into marcoroth:main Oct 17, 2025
13 checks passed
@timkaechele timkaechele deleted the lexer-peek-string-migration branch October 17, 2025 17:09
asilano pushed a commit to fac/herb that referenced this pull request Oct 21, 2025
This PR migrates the lexer peek helper interfaces to use `hb_string_T`
instead of null terminated strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants