codemirror/lang-python

Fork 0

Indentation for match-case statements (Python 3.10 - Structural Pattern Matching - PEP634, PEP635, PEP636) #6

Merged

deephbz merged 2 commits from feature/pep634-matchcase-indent into main

2025-01-18 12:35:03 +01:00

deephbz commented

2025-01-16 14:42:12 +01:00

(Migrated from github.com)

Lezer already supports the match-case grammer in Python3.10 PEP634, relevant commit is this. However, lang-python does not support indent for match case block yet.
This PR implements such support. Besides, I re-organized the python indent module with heavy documents to make the logic flow more straight-forward. There's no logging and the indent processing logic is recursive in nature. Such docs should help future development a lot.

Lezer already supports the match-case grammer in Python3.10 PEP634, relevant commit is [this](https://github.com/lezer-parser/python/commit/d370cbbef16693d6f4393ae1065e59eb160d2ac5#diff-f6ac40344cb9aef8b38768c84e1cd7ac89d1a11fe0bc24f1b65227c8d51ae9d4). However, lang-python does not support indent for match case block yet. This PR implements such support. Besides, I re-organized the python indent module with heavy documents to make the logic flow more straight-forward. There's no logging and the indent processing logic is recursive in nature. Such docs should help future development a lot.

krassowski (Migrated from github.com) reviewed

2025-01-16 14:42:12 +01:00

marijnh commented

2025-01-16 14:47:35 +01:00

(Migrated from github.com)

Besides, I re-organized the python indent module with heavy documents to make the logic flow more straight-forward.

That's not really a good idea—it forces me to review a bunch of spurious changes and reformats. Any way you could separate out the match case support without rewriting this entire file?

> Besides, I re-organized the python indent module with heavy documents to make the logic flow more straight-forward. That's not really a good idea—it forces me to review a bunch of spurious changes and reformats. Any way you could separate out the match case support without rewriting this entire file?

deephbz commented

2025-01-16 15:20:41 +01:00

(Migrated from github.com)

Besides, I re-organized the python indent module with heavy documents to make the logic flow more straight-forward.

That's not really a good idea—it forces me to review a bunch of spurious changes and reformats. Any way you could separate out the match case support without rewriting this entire file?

I've finished clean up and now the diff is more clear: only two match-case statements added to indentNodeProp, together with clear variable names (no logic changes) of the two small util functions.

I'm aware that working with legacy codes it's best to keep change small but the existing implementation of the function is kind-of tricky. I genuinely think making them easier (for future maintainers) to follow could be a right choice. Besides, extensive unit-tests should help ensure there's no degradation. I've tested old/new version on those tests.

> > Besides, I re-organized the python indent module with heavy documents to make the logic flow more straight-forward. > > That's not really a good idea—it forces me to review a bunch of spurious changes and reformats. Any way you could separate out the match case support without rewriting this entire file? I've finished clean up and now the diff is more clear: only two match-case statements added to `indentNodeProp`, together with clear variable names (no logic changes) of the two small util functions. I'm aware that working with legacy codes it's best to keep change small but the existing implementation of the function is kind-of tricky. I genuinely think making them easier (for future maintainers) to follow could be a right choice. Besides, extensive unit-tests should help ensure there's no degradation. I've tested old/new version on those tests.

krassowski (Migrated from github.com) reviewed

2025-01-16 15:47:11 +01:00

src/python.ts

Do this and deindentingPattern handle false positive case of case = 123? This is because case is a soft keyword and can be used as a variable name (compared to except, elif, etc).

Do this and `deindentingPattern` handle false positive case of `case = 123`? This is because `case` is a soft keyword and can be used as a variable name (compared to `except`, `elif`, etc).

deephbz (Migrated from github.com) reviewed

2025-01-16 17:07:54 +01:00

src/python.ts

Good catch. I've updated so that it only trigger "indentOnInput" action and properly de-indent following skw<case> grammer, like in below example:

print("Hello world")
match x:
  case 2:  # match case
      case = 1 # case as variable name

Rationale is that as of typing to

print("Hello world")
match x:
  case 1:
    case 
# you can't know if this typing goes to 
    case 2:
# or goes to 
    case = 8

Therefore, we might need to change the de-indenting pattern regex from simply "case " with a trailing space to "case :"

Good catch. I've updated so that it only trigger "indentOnInput" action and properly de-indent following `skw<case>` grammer, like in below example: ```py print("Hello world") match x: case 2: # match case case = 1 # case as variable name ``` Rationale is that as of typing to ```python print("Hello world") match x: case 1: case # you can't know if this typing goes to case 2: # or goes to case = 8 ``` Therefore, we might need to change the de-indenting pattern regex from simply "case " with a trailing space to "case <word>:"

marijnh commented

2025-01-17 12:19:45 +01:00

(Migrated from github.com)

You're still adding semicolons (in a project that doesn't use them), changing let to const, and making other spurious changes. This is a bit like if I'd come into your house to arrange the furniture so that it looks the way I like—this being your house, I shouldn't come in an rearrange stuff to my taste.

I'm aware that working with legacy codes

This package is not legacy code.

You're still adding semicolons (in a project that doesn't use them), changing `let` to `const`, and making other spurious changes. This is a bit like if I'd come into your house to arrange the furniture so that it looks the way I like—this being your house, I shouldn't come in an rearrange stuff to my taste. > I'm aware that working with legacy codes This package is not legacy code.

deephbz commented

2025-01-17 15:02:45 +01:00

(Migrated from github.com)

You're still adding semicolons (in a project that doesn't use them), changing let to const, and making other spurious changes. This is a bit like if I'd come into your house to arrange the furniture so that it looks the way I like—this being your house, I shouldn't come in an rearrange stuff to my taste.

I'm aware that working with legacy codes

This package is not legacy code.

Thanks for the clarification. Apologize for dramatic change earlier.
Updated PR so it's now a single, isolated change focusing on the PEP634 feature itself.

> You're still adding semicolons (in a project that doesn't use them), changing `let` to `const`, and making other spurious changes. This is a bit like if I'd come into your house to arrange the furniture so that it looks the way I like—this being your house, I shouldn't come in an rearrange stuff to my taste. > > > I'm aware that working with legacy codes > > This package is not legacy code. Thanks for the clarification. Apologize for dramatic change earlier. Updated PR so it's now a single, isolated change focusing on the PEP634 feature itself.

marijnh commented

2025-01-18 12:34:45 +01:00

(Migrated from github.com)

Nice! This looks like a solid, clean pull request now. Thanks for implementing that. Merging.

🎉 1

No reviewers

krassowski

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

codemirror/lang-python!6

No description provided.

Rows
Columns