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.

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

Accept a TextMate scope selector in bufferRangeForScopeAtPosition #18015

Merged
merged 1 commit into from Sep 11, 2018

Conversation

Projects
None yet
4 participants
@maxbrunsfeld
Copy link
Contributor

maxbrunsfeld commented Sep 11, 2018

@t9md

This comment has been minimized.

Copy link
Contributor

t9md commented Sep 11, 2018

state

Thanks, I tried your branch and I could select inline-block comment by comment.block scope.
But still have some minor diff.
Above is this branch's behavior. Double slash line comment starts with // can also be selected by comment.block.
Which was NOT in TextMate parser(it doesn't return the range of line-comment).
TreeSitter does not distinguish line-comment and inline-block-comment.

I think this is as designed.
Can't I expect 100% of scope compatibility when switching to TreeSitter?
For this one, previous behavior is better since it allows a granular level of control to the package author.

@Arcanemagus

This comment has been minimized.

Copy link
Contributor

Arcanemagus commented Sep 11, 2018

Can't I expect 100% of scope compatibility when switching to TreeSitter?

They are different parsing systems, so they likely won't give you identical results. In many cases Tree-sitter will likely be better at parsing things since it isn't relying on a regex parse of single lines, however it is still relatively new so there may be rough edges that still need to be ironed out.

@t9md

This comment has been minimized.

Copy link
Contributor

t9md commented Sep 11, 2018

I'm getting to understand it.

TreeSitter parses code more correctly and consistently without depending on error-prone regex matching.

By that, tree-sitter doesn't distinguish function name between one used in declaration and one call(invocation).
Also doesn't distinguish between inline-comment and line-comment, it's all just a "comment".
So we see same syntax color whenever function name appears.

Although it was error-prone, regex-based parsing also provided more class names(since a variety of regex matchings are directly converted to the class names used in CSS), it can be used when matching to a very specific context using a scope name.

As my understanding, this approach is no longer feasible in tree-sitter. SInce tree-sitter doesn't make unnecessary distinguishment which was the natural consequence where regex-based parsing is exercised in old parsing mechanism.

@Aerijo

This comment has been minimized.

Copy link
Member

Aerijo commented Sep 11, 2018

tree-sitter doesn't distinguish function name used in declaration and function call

It can. I believe your talking about the translation to scopes... maybe.

Also doesn't distinguish inline-comment or line-comment, it's all just a "comment".

Again, it can. It just appears that the grammar writer did not decide to do this. If you make a case for why it would help, I'm sure changes will be considered.

regex-based parsing also provided more class names(since a variety of regex matchings are directly converted to the class names used in CSS), it can be used when matching to a very specific context using a scope name

Well, if you judge Tree-sitter by the scopes it provides, then yeah, probably. My understanding is you're supposed to interact with the nodes of a Tree-sitter grammar, and not so much the scopes. Of course you can work with scopes, but as you say they seem to be lacking compared to TextMate grammars. This will be improved as people raise issues and make PRs though.

SInce tree-sitter doesn't make unnecessary distinguishment which was the natural consequence where regex-based parsing is exercised in old parsing mechanism.

I'm not sure what you mean here.

@t9md

This comment has been minimized.

Copy link
Contributor

t9md commented Sep 11, 2018

@Aerijo I understand what you explained, thanks.
Yeah, I agree with your suggestion, maybe I'd better work on the nodes than the scopes.
But that means I need to do lots of work to migrate to tree-sitter in my vim-mode-plus package.
So I tried to find the way to creating the shim which allows old code works without making big changes.

Anyway, I'll continue to dig further to get me to understand the changes.

Thanks.

@Aerijo

This comment has been minimized.

Copy link
Member

Aerijo commented Sep 11, 2018

@t9md

So I tried to find the way to creating the shim which allows old code works without making big changes.

Sudden changes are difficult to keep up with, especially new ones where the interface could be unstable (it appears to be settling down though; mostly just added features now). There will be many packages that never move to Tree-sitter, and will rely on the scope translations.

So I think you can use scopes, but might need to raise issues and make PRs on the relevant packages to get them there. But in the long term, I'd look for how to use Tree-sitter nodes.

@maxbrunsfeld maxbrunsfeld merged commit a9d2041 into master Sep 11, 2018

2 of 3 checks passed

Atom Pull Requests #20180911.1 failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maxbrunsfeld maxbrunsfeld deleted the mb-buffer-range-for-scope-fixes branch Sep 11, 2018

@maxbrunsfeld

This comment has been minimized.

Copy link
Contributor Author

maxbrunsfeld commented Sep 11, 2018

@t9md Yeah, I'm still figuring out what we need to do for packages to transition. Basically, Tree-sitter can provide you much more information about the code than TextMate could, but the data is structured differently - in a syntax tree instead of in a list of scopes.

The problem with scopes is that if there are a lot of nested scopes, they must all be rendered to the DOM as a series of nested spans, which has a steep performance cost during rendering. So initially, when introducing Tree-sitter and designing the scope mapping system for theme compatibility, I wanted to produce the most minimal set of scopes that would be needed for themes to work.

For example, as you mentioned, we don't produce shims for scopes like meta.method-call.js. I believed that this was ok because those scopes were already quite unreliable with TextMate. For example, adding or removing a \n character can change meta.method-call.js into meta.function-call.js, or completely remove a scope like meta.function.definition.js.

I think that if you do want to make more sophisticated queries of syntactic structure, Tree-sitter should be more useful in the long term, but you'll need to use different APIs. Currently, the new APIs are not documented, because I haven't fully decided which ones we should commit to. Maybe if you get some time, you could let me know which APIs would be useful to you. I would think that Vim-mode-plus's text object system might be a good use case for these new syntax tree APIs. You could try something like this:

const languageMode = editor.getBuffer().getLanguageMode()

let range
if (languageMode.getSyntaxNodeAtPosition) {
  // new code

  const node = languageMode.getSyntaxNodeAtPosition(position)

  // The names of these nodes will depend on the grammar
  const functionNode = node.closest(['function', 'arrow_function', 'method_definition'])

  range = functionNode.range

} else {
  // old code
}

Sorry that this isn't all figured out yet; I know you might not have time to dive into this stuff. But in case you do, here are some things that might be useful to look at:

@t9md

This comment has been minimized.

Copy link
Contributor

t9md commented Sep 11, 2018

Thanks for the elaborate explanation, I didn't know that the flight-manual have such a detailed explanation of new tree-sitter.
I've already checked some example grammars and also your talk on YouTube.

Anyway, I completely agree that the tree-sitter way is more reliable and give more structured information into code.
Nice work and thank you for creating such an ambitious project(I knew tree-sitter from a few years ago when I have no idea what it goes.)

Your example code for text-object is very helpful, I've already been trying similar thing as well here.

If I have reached to clear understanding for API I need, I'll tell you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.