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

feat(yaml): lexer that can lex very simple examples #3430

Conversation

dyc3
Copy link
Contributor

@dyc3 dyc3 commented Jul 13, 2024

Summary

I figured I'd take a stab at building the lexer based on the grammar in #3400. It's somewhat inspired by the json lexer. I'm posting it early here so people can see it, and avoid duplicated effort.

related to: #2365

Test Plan

Added some unit tests, and they all pass.

@github-actions github-actions bot added the A-Tooling Area: internal tools label Jul 13, 2024
Copy link

codspeed-hq bot commented Jul 13, 2024

CodSpeed Performance Report

Merging #3430 will not alter performance

Comparing dyc3:07-13-feat_yaml_lexer_that_can_lex_very_simple_examples (b143aed) with main (88f1d5e)

Summary

✅ 101 untouched benchmarks

@dyc3 dyc3 mentioned this pull request Jul 13, 2024
6 tasks
@dyc3 dyc3 force-pushed the 07-13-feat_yaml_lexer_that_can_lex_very_simple_examples branch 2 times, most recently from 9dff440 to 3e0cca2 Compare July 13, 2024 21:36
@github-actions github-actions bot removed the A-Tooling Area: internal tools label Jul 13, 2024
Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

The JSON lexer is old and it hasn't been refactored yet.

In the meantime, we created a proper infrastructure for creating a lexer, and this is documented in the CONTRIBUTING.md file of the biome_parser crate. It is possible to reach this document from the main CONTRIBUTING.md, at the root of the repository.

The new infrastructure gives you many methods that you don't need to implement again, e.g. peek_byte, consume_whitespace

Also, avoid unchecked methods whenever possible.

crates/biome_yaml_parser/src/lexer/mod.rs Outdated Show resolved Hide resolved
crates/biome_yaml_parser/src/lexer/mod.rs Outdated Show resolved Hide resolved
@dyc3
Copy link
Contributor Author

dyc3 commented Jul 14, 2024

in the CONTRIBUTING.md file of the biome_parser crate.

Yeah that's the document I was referencing when writing this. That, and looking at the json lexer. Is there anything more than the "Implement a lexer" section of that doc? It's a little sparse on info.

@dyc3 dyc3 force-pushed the 07-13-feat_yaml_lexer_that_can_lex_very_simple_examples branch from 3e0cca2 to c7a1847 Compare July 14, 2024 11:48
@ematipico
Copy link
Member

Unfortunately that part still lacks some documentation due to some recent refactors. I completely forgot to follow up and add more to it

@dyc3 dyc3 force-pushed the 07-13-feat_yaml_lexer_that_can_lex_very_simple_examples branch 3 times, most recently from 9885aa9 to bf96f27 Compare July 14, 2024 13:46
@ematipico
Copy link
Member

I suggest looking at the GraphQL parser and tokenizer. It's the most recent one

@Netail
Copy link
Contributor

Netail commented Aug 27, 2024

@dyc3 any progress? 😅

@dyc3
Copy link
Contributor Author

dyc3 commented Aug 27, 2024

I haven't had time recently, but I could try and get this merged this week.

@dyc3 dyc3 force-pushed the 07-13-feat_yaml_lexer_that_can_lex_very_simple_examples branch from bf96f27 to 2462a59 Compare August 27, 2024 12:22
@dyc3 dyc3 marked this pull request as ready for review August 27, 2024 12:28
@dyc3 dyc3 requested a review from ematipico August 27, 2024 12:28
@dyc3 dyc3 force-pushed the 07-13-feat_yaml_lexer_that_can_lex_very_simple_examples branch from 2462a59 to b143aed Compare August 27, 2024 12:29
@ematipico ematipico merged commit b16def6 into biomejs:main Aug 27, 2024
12 checks passed
@dyc3 dyc3 deleted the 07-13-feat_yaml_lexer_that_can_lex_very_simple_examples branch August 27, 2024 14:12
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.

3 participants