Remove findEnclosingTags method

50Wliu commented 5 years ago

Requirements

Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
All new code requires tests to ensure against regressions

Description of the Change

findEnclosingTags had an important limitation: it checked TextMate scopes, and therefore was affected by the tokenization limit. Most code paths already used findStartEndTags instead, which skipped the scope check. The last place that used it also hadn't been updated to support the new syntax tree method of finding tags. Therefore, this PR:

Updates the last codepath to remove its dependence on findEnclosingTags in favor of findStartEndTags and also uses that only as a fallback if tree-sitter grammars aren't being used.
Removes findEnclosingTags completely.

Alternate Designs

N/A

Benefits

bracket-matcher:go-to-matching-bracket works on HTML files when tree-sitter is enabled
bracket-matcher:go-to-matching-bracket does not throw an error when there are no enclosing brackets (#380)
bracket-matcher:go-to-matching-bracket works on very long lines when tree-sitter is disabled

Possible Drawbacks

When tree-sitter is disabled, it may be possible to find a tag lookalike that isn't actually a tag since no scope checks are performed.

Verification Process

Tested manually and confirmed that bracket-matcher:go-to-matching-bracket no longer throws when there are no enclosing brackets. The included test fails without the included changes.

Applicable Issues

Fixes #380

alexchandel commented 5 years ago

This issue has generated multiple reports on atom/atom#18160 recently, and yet despite multiple intervening Atom updates, it still has not been merged into Atom.

Aerijo commented 5 years ago

@alexchandel The "merged" description begs to differ. Note it was only merged a few days ago, so some patience is necessary before it makes it's way to into releases. But, theoretically, you would get this fix if you were to clone and build the repo in its current state (you'd also get all the other unreleased changes which have not been tested even in beta yet, so its not really recommended though).

You didn't specify which version of Atom you use, but this change will first be in beta, and then normally stable will get it a minor release later.

50Wliu commented 5 years ago

I did forget to publish a new version. I'll do that today.

atom / bracket-matcher