tree-sitter-python icon indicating copy to clipboard operation
tree-sitter-python copied to clipboard

perf!: don't look for indent/dedent if comment is at current indent level

Open llllvvuu opened this issue 1 year ago • 1 comments

Whether an indent/dedent token is generated for a comment depends on whether the indent/dedent is persisted after the comment. This takes O(comment_length) to check. Checking it for every line takes O(comment_length * comment_lines), which is quadratic.

An optimization was made in a901729 which skips this check when there is no possible indent/dedent, such as in the following:

# comment 1...
# comment 1000
print("foo")

However, the check cannot be skipped in the following case:

class Foo:
    def foo():
        print("bar")
    # comment 1...
    # comment 1000
    def bar():
        print("foo")

which comes up when commenting out code.

This PR optimizes this case by skipping the check if the comment is at the current indent length. In the above example, # comment 1 looks ahead to def bar() and changes the indent length to 4, after which comments 2-1000 do not look ahead, since they are also at indent length 4.

This PR does not optimize the following case:

class Foo:
    def foo():
        print("bar")
    # comment 1...
    # comment 1000
        print("foo")

but it does optimize:

class Foo:
    def foo():
        print("bar")
        # comment 1...
        # comment 1000
        print("foo")

BREAKING CHANGE: In the following scenario, we previously generated an indent token before comment 1; we now generate it before comment 2. This is more consistent with how dedents are handled.

def foo():
# comment 1
   # comment 2
   print("bar")

Related: https://github.com/nvim-treesitter/nvim-treesitter/issues/4839

llllvvuu avatar Sep 22 '23 05:09 llllvvuu

What's interesting is that before the 188b6b06 commit it seems there was no such problem with comments, so maybe it worth to looking for an alternative solution and try to remove comments from externals.

ahlinc avatar Sep 27 '23 08:09 ahlinc