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.

Sign up for GitHub

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

Jump to bottom

Re-order lexer methods #11716

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Re-order lexer methods #11716

merged 1 commit into from
Jun 3, 2024

Conversation

dhruvmanila
Copy link
Member

Summary

This PR re-orders the lexer methods in the following order:

  1. next_token
  2. lex_token
  3. eat_indentation
  4. handle_indentation
  5. skip_whitespace
  6. consume_ascii_character
  7. try_single_char_prefix
  8. try_double_char_prefix
  9. lex_identifier
  10. lex_fstring_start
  11. lex_fstring_middle_or_end
  12. lex_string
  13. lex_number
  14. lex_number_radix
  15. lex_decimal_number
  16. radix_run
  17. lex_comment
  18. lex_ipython_escape_command
  19. consume_end

Following was considered for the ordering:

  • 1 is the main entry point which delegates to 2
  • 3, 4, 5 are all related to whitespace which is done first
  • 6 is the entrypoint for an ascii character which delegates to 9, 12, 13, 17, 18, 19
  • Others are grouped around similar kind of methods

@dhruvmanila dhruvmanila added the internal An internal refactor or improvement label Jun 3, 2024
@dhruvmanila dhruvmanila requested a review from MichaReiser as a code owner June 3, 2024 09:06
MichaReiser
MichaReiser approved these changes Jun 3, 2024
View reviewed changes
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thank you.

I would consider reordering:

  • 7, 8, 9: Make 9 the first method. Similar to consume_ascii_character that later calls into the specific lexing methods, lex_identifier calls into 8 or 9. This also aligns it, I think, with the number lexing
  • 10, 11, 12: I'm not familiar enough to know this from top of my head but I would make 12 come first if it calls into 10 and/or 11.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 3, 2024 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@dhruvmanila dhruvmanila force-pushed the dhruv/reorder-lexer-methods branch from a92708c to 0578b17 Compare June 3, 2024 12:22
Base automatically changed from dhruv/parser-phase-2 to main June 3, 2024 12:53
@dhruvmanila dhruvmanila force-pushed the dhruv/reorder-lexer-methods branch from 0578b17 to 9bd4249 Compare June 3, 2024 12:54
@dhruvmanila dhruvmanila enabled auto-merge (squash) June 3, 2024 12:55
@dhruvmanila dhruvmanila merged commit 4a155e2 into main Jun 3, 2024
16 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/reorder-lexer-methods branch June 3, 2024 12:58
carljm added a commit that referenced this pull request Jun 3, 2024
@carljm
Merge branch 'main' into cjm/cfg1
b1f6153 
* main: (25 commits)
  Isolate non-breaking whitespace indentation test case (#11721)
  Generator should add a newline before type statement (#11720)
  Remove less used parser dependencies (#11718)
  Use string expression for parsing type annotation (#11717)
  Re-order lexer methods (#11716)
  Maintain synchronicity between the lexer and the parser (#11457)
  Update NPM Development dependencies (#11713)
  Update pre-commit dependencies (#11712)
  Update cloudflare/wrangler-action action to v3.6.1 (#11709)
  Update dependency monaco-editor to ^0.49.0 (#11710)
  Update Rust crate tracing-tree to v0.3.1 (#11703)
  Update Rust crate libcst to v1.4.0 (#11707)
  Update Rust crate itertools to 0.13.0 (#11706)
  Update Rust crate insta to v1.39.0 (#11705)
  Update Rust crate proc-macro2 to v1.0.85 (#11700)
  Update Rust crate toml to v0.8.13 (#11702)
  Update Rust crate strum_macros to v0.26.3 (#11701)
  Update UP035 for Python 3.13 and the latest version of typing_extensions (#11693)
  Add RDJson support. (#11682)
  [`pyupgrade`] Write empty string in lieu of panic (#11696)
  ...
carljm added a commit that referenced this pull request Jun 3, 2024
@carljm
Merge branch 'cjm/cfg1' into cjm/cfg2
3dbf5d7 
* cjm/cfg1: (26 commits)
  review comments
  Isolate non-breaking whitespace indentation test case (#11721)
  Generator should add a newline before type statement (#11720)
  Remove less used parser dependencies (#11718)
  Use string expression for parsing type annotation (#11717)
  Re-order lexer methods (#11716)
  Maintain synchronicity between the lexer and the parser (#11457)
  Update NPM Development dependencies (#11713)
  Update pre-commit dependencies (#11712)
  Update cloudflare/wrangler-action action to v3.6.1 (#11709)
  Update dependency monaco-editor to ^0.49.0 (#11710)
  Update Rust crate tracing-tree to v0.3.1 (#11703)
  Update Rust crate libcst to v1.4.0 (#11707)
  Update Rust crate itertools to 0.13.0 (#11706)
  Update Rust crate insta to v1.39.0 (#11705)
  Update Rust crate proc-macro2 to v1.0.85 (#11700)
  Update Rust crate toml to v0.8.13 (#11702)
  Update Rust crate strum_macros to v0.26.3 (#11701)
  Update UP035 for Python 3.13 and the latest version of typing_extensions (#11693)
  Add RDJson support. (#11682)
  ...
carljm added a commit that referenced this pull request Jun 3, 2024
@carljm
Merge branch 'cjm/cfg2' into cjm/cfg3
6a3780e 
* cjm/cfg2: (27 commits)
  review comments
  review comments
  Isolate non-breaking whitespace indentation test case (#11721)
  Generator should add a newline before type statement (#11720)
  Remove less used parser dependencies (#11718)
  Use string expression for parsing type annotation (#11717)
  Re-order lexer methods (#11716)
  Maintain synchronicity between the lexer and the parser (#11457)
  Update NPM Development dependencies (#11713)
  Update pre-commit dependencies (#11712)
  Update cloudflare/wrangler-action action to v3.6.1 (#11709)
  Update dependency monaco-editor to ^0.49.0 (#11710)
  Update Rust crate tracing-tree to v0.3.1 (#11703)
  Update Rust crate libcst to v1.4.0 (#11707)
  Update Rust crate itertools to 0.13.0 (#11706)
  Update Rust crate insta to v1.39.0 (#11705)
  Update Rust crate proc-macro2 to v1.0.85 (#11700)
  Update Rust crate toml to v0.8.13 (#11702)
  Update Rust crate strum_macros to v0.26.3 (#11701)
  Update UP035 for Python 3.13 and the latest version of typing_extensions (#11693)
  ...
carljm added a commit that referenced this pull request Jun 3, 2024
@carljm
Merge branch 'cjm/cfg3' into cjm/cfg4
fe878d2 
* cjm/cfg3: (29 commits)
  [red-knot] infer_symbol_public_type infers union of all definitions (#11669)
  review comments
  review comments
  review comments
  Isolate non-breaking whitespace indentation test case (#11721)
  Generator should add a newline before type statement (#11720)
  Remove less used parser dependencies (#11718)
  Use string expression for parsing type annotation (#11717)
  Re-order lexer methods (#11716)
  Maintain synchronicity between the lexer and the parser (#11457)
  Update NPM Development dependencies (#11713)
  Update pre-commit dependencies (#11712)
  Update cloudflare/wrangler-action action to v3.6.1 (#11709)
  Update dependency monaco-editor to ^0.49.0 (#11710)
  Update Rust crate tracing-tree to v0.3.1 (#11703)
  Update Rust crate libcst to v1.4.0 (#11707)
  Update Rust crate itertools to 0.13.0 (#11706)
  Update Rust crate insta to v1.39.0 (#11705)
  Update Rust crate proc-macro2 to v1.0.85 (#11700)
  Update Rust crate toml to v0.8.13 (#11702)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees
No one assigned
Labels
internal An internal refactor or improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dhruvmanila @MichaReiser