Skip to content

Rationale behind identifier/selector #19

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

Open
mizlan opened this issue Jun 3, 2021 · 9 comments
Open

Rationale behind identifier/selector #19

mizlan opened this issue Jun 3, 2021 · 9 comments

Comments

@mizlan
Copy link

mizlan commented Jun 3, 2021

Hi, I was a bit confused by the tree generated by the following code:

void main() {
  f(a.b);
}

The arguments node for f contains the following, when dumped using Neovim's treesitter playground:

arguments [1, 3] - [1, 8]
  identifier [1, 4] - [1, 5]
  selector [1, 5] - [1, 7]
    assignable_selector [1, 5] - [1, 7]
      unconditional_assignable_selector [1, 5] - [1, 7]
        identifier [1, 6] - [1, 7]

As you can see, it looks like there are two arguments to this function. Indeed, the identifier-selector as sibling nodes without a containing parent node happens universally throughout Dart code. I would imagine that it would make sense for there to be a "wrapper" node that encloses the identifier and following selectors, but there isn't. Is there a plan to add that to the grammar?

@mizlan
Copy link
Author

mizlan commented Jun 3, 2021

For reference, I have a plugin that allows for arbitrary children of an arguments node to be swapped (relevant thread here) but this identifier/selector behavior is interfering with the identification of function arguments.

@TimWhiting
Copy link
Collaborator

This is because a selector is not part of an identifier. This grammar tries to mimic the official dart spec as much as possible to make it easier to maintain, but I agree that it would make sense to group those nodes under another meta node. Feel free to take a look at the grammar and submit a pull request. I can't guarantee that I have time to look at it /change it right away, but I could probably spare the time to review a pull request.

@akinsho
Copy link
Collaborator

akinsho commented Jun 4, 2021

Hi @TimWhiting well since I basically started this whole train off with my PR to iswap.nvim I can look into contributing the necessary changes to the grammar. I'd really appreciate a bit of advice on how to proceed though since I've had a look through the grammar and am not really sure how to make the change?

Would this be scoped to all nodes that have a selector where the selector is treated as a sibling or just things like arguments and lists etc. since the same probably likely exists wherever a selector is used. Also do you know where they are being set as siblings that I can wrap?

@TimWhiting
Copy link
Collaborator

TimWhiting commented Jun 4, 2021

In general arguments will be some type of expression. Currently most expression types are hidden nodes, because they can get quite deeply nested. I've added an argument node underneath arguments. So now it can either be argument or named_argument underneath arguments, and the argument / named argument encapsulates the expression. I'm guessing there are other places in the grammar where the expression node is hidden and it could use some more aptly-named node to partition things. Take a look at the latest commit (7d7fa53) at the grammar.js file to see what I did, and you can do the same elsewhere in the grammar. You can ignore the stuff I changed with the 'assignable_selector'. I just cleaned up that node since it can only be an unconditional_assignable_selector or conditional_assignable_selector, so that node was just adding noise.

@TimWhiting
Copy link
Collaborator

I'll leave this issue open since I believe there are other places in the grammar that just use $._expression which is hidden, and could use more contextual information exposed in the tree. @akinsho, thanks for the offer, please feel free to improve the grammar and submit PRs.

@akinsho
Copy link
Collaborator

akinsho commented Jun 4, 2021

@TimWhiting thanks a bunch for adding that 👍🏾 I'll have a look to see if I encounter any other areas that require similar treatment and use the example of your commit if I find any.

@mizlan
Copy link
Author

mizlan commented Jun 4, 2021 via email

@textzenith
Copy link

textzenith commented Feb 27, 2023

Thanks, everyone, for your great work in creating a Dart tree-sitter grammar—seems like an arduous task!

Because I want to use tree-sitter to rearrange trees of Flutter widgets (i.e. identifer / selector node pairs), this behaviour is causing a lot of problems. I would like to offer to fix it, but I need to improve my TS-fu first. 😏

(As always, it seems like one of the hardest problems for a programmer is naming things—what's the parent node name that everyone would be able to agree on?)

@TimWhiting
Copy link
Collaborator

I took a closer look at this issue the other day, and was having a bit of trouble reorganizing. I'll give it a try again sometime soon now that I've added the dart 3.0 features, I'm moving on to cleaning up the grammar. If you want to start contributing though I'd be happy to review a PR. I would say even just making the _postfix_expression not hidden for selectors would be a good start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants