-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
fix: editing subtree that depends-on-column: ... #3218
Conversation
…ues: before and after parse-tree structures should be the same The edit in the test is just changing an identifier from "c" to "c1234". Such edit should not affect the resulting parse tree's structure (the parse tree's S-expression should stay the same).
…ased on the edit's end-column
…update strings-read assertion After the fix on ts_subtree_edit(), fewer characters are read
… is not (move the check to per-child)
…rder now reflects the edit correctly (just the identifier being renamed)
Hi, I don't know what behavior this new code would implement, but the existing behavior is what we want, I believe. The existing behavior is that if the external scanner's |
I'm going to close this out because I believe the existing test is correct, and your change to the test would break the desired behavior. If I'm missing some more subtle reason why this invalidation logic should be changed, then feel free to explain further, and we can reopen. |
Alright, I have reworked my PR so it's much simpler now -- only adding two checks and nothing else (no test change, and all the tests still pass). master...rooney:tree-sitter:col-dependent-fix The change is needed because there are some cases where a token-that-depends-on-column should be invalidated but isn't. Let me demo it using a simplest-as-possible example: // grammar.js
rules: {
source_file: ($) => seq(/[ \t\r\n]+/, choice($.odd, $.even), $.x),
x: () => "x",
},
externals: ($) => [$.odd, $.even], Here, our grammar will: 1. consume leading whitespace, 2. consult the external scanner to see if the column is at odd/even position, and 3. match "x". (the scanner is stateless): // scanner.c
enum TokenType { ODD, EVEN };
bool tree_sitter_coldemo_external_scanner_scan(void *payload, TSLexer *lexer,
const bool *valid_symbols) {
lexer->result_symbol = lexer->get_column(lexer) % 2 ? ODD : EVEN;
return true;
} Now, here's how the playground goes on And here's how the playground goes after the two-line patch: tree-sitter-coldemo.zip (contains only To be extra sure, I also have checked this new patch against the complete tree-sitter-haskell -- all tests there still pass running this patch. |
Thanks for the response, and nice work. As part of your branch, can you add your new example scanner as a test to the Tree-sitter test suite? |
Reopening. |
Thanks @maxbrunsfeld
Also, added an optimization for when the edits don't actually move the columns. (The test for this optimization checks that there's fewer bytes being read by the recorder). Also have re-run tree-sitter-haskell |
I don't think it can't be reopened, can you open another PR @rooney? |
Oh sorry, thought that I reopened it. The GitHub UI mislead me a bit there. |
sure, np, created another PR #3257 |
... invalidation should be based on the edit's end-column.
Fix #3217