Adding Rust Support #546

trace-andreason · 2022-02-04T23:09:21Z

What

Adds support for the rust programming language

Checklist

trace-andreason · 2022-02-19T21:42:38Z

@pokey think this PR is in a pretty good spot now.

pokey

This is looking really good. Left a few inline comments. In addition, I've taken the liberty of editing the description to use the PR template for adding a language. It would be great if you could make sure you check all the boxes there

In particular, I see that you've used "chuck" for all your tests, but we generally use that one just when the removal is interesting, eg it needs to clean up a , or :. We use "change" test (or "clear", if you're still using that spoken form) as the default action for tests. Otherwise you're basically just testing our whitespace removal code in addition to your intended test case 😅

I don't think you need to re-record all your test cases, but for the ones where removal is interesting, (eg "arg", "type", etc), it would be good to add a "change" test as well

Also:

src/languages/rust.ts

pokey · 2022-02-22T11:51:52Z

src/languages/rust.ts

+
+  // Generated by the following command:
+  // `curl https://raw.githubusercontent.com/tree-sitter/tree-sitter-rust/master/src/node-types.json | jq '[.[] | select(.type == "_declaration_statement" or .type == "_expression") | .subtypes[].type]'`
+  const STATEMENT_TYPES = [


This is going to be way too broad. You'll prob want to just look for children of _declaration_statement, and then add expression_statement to the list. You don't want to get trapped in an expression when you say "take state"

I think _declaration_statement will be too narrow, it wouldn't include if statements for example

Got it. Looks like the Rust parser is like Ruby, where the expression_statement node doesn't appear in the parse tree. The problem is that we need to distinguish between eg an if statement as a statement, vs if statement as an expression. For example, when you say "take state", you'll capture the if statement here:

let foo = if bar { "hello" } else { "there" }

which you don't want, because that if statement is actually functioning as an expression. In Ruby, we resorted to inspecting the parent node to see if the expression is at statement level vs a nested expression. See https://github.com/cursorless-dev/cursorless-vscode/blob/993fbc8d14846414dbfc374c6fa56d3098f03465/src/languages/ruby.ts#L157-L166. Should be able to do something similar here

ahhhh I see what you are saying now

src/languages/rust.ts

pokey · 2022-02-22T11:59:45Z

src/languages/rust.ts

+      "let_declaration.identifier!",
+      "parameter.identifier!",
+    ],
+   };


Looks like you're missing a few scope types here:

class. Might also want to include structs in this one

item. This is for list elements as well as pairs in a map

map

value. This one is for RHS of an assignment, as well as value in a map

key

Maybe a few others? Worth looking at eg Typescript impl for inspiration here. I think the other Rust PR might have a couple of these as well?

the map/key/value thing doesn't really exist in rust as far as I know: https://doc.rust-lang.org/book/ch08-03-hash-maps.html. Given the syntax, it would probably make more sense to just target key, values as args.

items are proving hard to implement...

I added class, I'm not sure I can actually implement item though

Re map/key/value, fair enough; let's leave that out

Re item, did argumentMatcher("array_expression") not work?

Also, although there are no "values" in the map sense of the word, you'll still want "value" as right-hand side of assignment, default value of parameter, return value of function, etc

src/languages/rust.ts

pokey · 2022-03-18T14:35:40Z

src/test/suite/fixtures/recorded/languages/rust/changeState.yml

+initialState:
+  documentContents: |-
+    let expensive_closure = |num| {
+        println!("calculating slowly...");


This test is nice. Worth also testing that "change state" with cursor inside calculating clears this line exactly

pokey · 2022-05-30T13:33:14Z

@trace-andreason shall I take this one to the finish line? I think it's getting pretty close

purpleP · 2022-06-11T20:28:24Z

@pokey

Hi, I wanted to continue the work on this. I’ve done a rebase and tried to run this, but instructions aren’t clear to me. This is what I see when I try to run the extension.

out.mp4

pokey · 2022-06-11T22:51:18Z

Hi @purpleP great to hear you are interested in picking it back up!

It's a bit tough to tell what's happening in your video, but you will probably want to run step 3 from https://www.cursorless.org/docs/contributing/#steps if you haven't already

purpleP · 2022-06-12T07:32:40Z

Hi @purpleP great to hear you are interested in picking it back up!

It's a bit tough to tell what's happening in your video, but you will probably want to run step 3 from https://www.cursorless.org/docs/contributing/#steps if you haven't already

Oh, yeah. I forgot to say that it gives me an error about fs/promises not being installed.

pokey · 2022-06-12T07:34:03Z

Strange. Does that also happen on main?

purpleP · 2022-06-12T10:05:24Z

Strange. Does that also happen on main?

Yep.

pokey · 2022-06-12T10:08:51Z

Huh. Maybe remove your node_modules and repeat the incantation I linked above?

auscompgeek · 2022-06-12T10:32:14Z

fs/promises was added in Node 14. Any particular reason for using an EOL version of Node?

purpleP · 2022-06-12T12:04:45Z

fs/promises was added in Node 14. Any particular reason for using an EOL version of Node?

Thanks, updating to 16 LTS helped.

purpleP · 2022-06-14T17:22:33Z

@pokey How do I match a function call?

Right now cursorless select the whole call_expression node and I want it to select field_identifier with all arguments.

pokey · 2022-06-14T21:14:00Z

That's actually considered correct behaviour for cursorless "call" scope type. I believe what you're after is #707. That should probably happen as a separate PR so that we can introduce it in a few languages for consistency

purpleP · 2022-06-16T09:52:07Z

@pokey

What should I do with type constructors and type arguments? I think they should work the same way functions and function arguments work.

Also what should I do when I want to delete things like mut modifier? Removing just the token leaves trailing space.

pokey · 2022-06-17T06:25:31Z

What should I do with type constructors and type arguments? I think they should work the same way functions and function arguments work.

I think it's fine to support "arg" for those type arguments. But I wouldn't support "type" for them. That's just for the top-level one (green in your annotations). Be sure to clean up that arrow on "chuck" if you're not already

Also what should I do when I want to delete things like mut modifier? Removing just the token leaves trailing space.

Sorry not sure I'm following. Is this for "type", "arg", or something else?

purpleP · 2022-06-17T08:05:10Z

Sorry not sure I'm following. Is this for "type", "arg", or something else?

I’m talking about fn f(&**mut** self

auscompgeek · 2022-06-17T08:08:40Z

my 2c: in param: &mut Param, the type is &mut Param, and the argument is param: &mut Param. I'm not sure how you would select the type without modifiers though (i.e. Param), or just the modifiers though 🤔

auscompgeek · 2022-06-17T08:13:18Z

either way, rustfmt will clean up any lingering whitespace, right? probably not a huge problem for deleting mut in a borrow type

pokey · 2022-06-17T13:19:49Z

Re modifiers, a few thoughts:

I agree that modifiers should be considered part of the type, so eg
```
fn f(&mut self, param: &mut Param)
  // ^--^              ^--------^
```
And the removal range should include the trailing space or the leading colon, so for example:
```
fn f(&mut self, param: &mut Param)
  // ^---^           ^----------^
```
This should be accomplished by using a trailing delimiter of on &mut and a leading delimiter of : on &mut Param
When referring to mut as a token, today it won't clean up whitespace because it's a bit conservative about merging tokens, but we are considering changing it so that it will remove spaces if it detects that it won't merge tokens. Either way that is probably independent of rust support
It might be worth adding a new scope type called "modifier" that can be used to refer to something like mut. Alternately we maybe we could repurpose "attribute"

pokey · 2022-07-05T10:28:59Z

Finished in #775

trace-andreason and others added 3 commits December 19, 2021 13:15

initial commit

3cd57d9

add function call

9803dc2

wrapping things up

a7bf8c2

trace-andreason requested a review from pokey as a code owner February 4, 2022 23:09

trace-andreason mentioned this pull request Feb 4, 2022

Add rust support #540

Closed

1 task

trace-andreason added 2 commits February 19, 2022 13:39

combining ideas from other PR

95a3efe

Merge branch 'main' into main

feae6a6

trace-andreason changed the title ~~my adding rust support, for @purpleP to compare with his branch~~ Adding Rust Support Feb 21, 2022

pokey reviewed Feb 22, 2022

View reviewed changes

trace-andreason added 2 commits March 15, 2022 15:52

adding tests + struct things

9eb4a31

Merge branch 'main' into main

b47fa9d

pokey reviewed Mar 18, 2022

View reviewed changes

pokey mentioned this pull request Jun 20, 2022

Feature/rust support #775

Merged

14 tasks

pokey closed this Jul 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Rust Support #546

Adding Rust Support #546

trace-andreason commented Feb 4, 2022 •

edited by pokey

Loading

trace-andreason commented Feb 19, 2022

pokey left a comment •

edited

Loading

pokey Feb 22, 2022

trace-andreason Mar 15, 2022

pokey Mar 18, 2022

trace-andreason Mar 18, 2022

pokey Feb 22, 2022 •

edited

Loading

trace-andreason Mar 15, 2022

trace-andreason Mar 15, 2022

trace-andreason Mar 15, 2022

pokey Mar 18, 2022 •

edited

Loading

pokey Mar 18, 2022

pokey Mar 18, 2022 •

edited

Loading

pokey commented May 30, 2022

purpleP commented Jun 11, 2022 •

edited

Loading

pokey commented Jun 11, 2022

purpleP commented Jun 12, 2022 •

edited

Loading

pokey commented Jun 12, 2022

purpleP commented Jun 12, 2022

pokey commented Jun 12, 2022

auscompgeek commented Jun 12, 2022

purpleP commented Jun 12, 2022

purpleP commented Jun 14, 2022 •

edited

Loading

pokey commented Jun 14, 2022

purpleP commented Jun 16, 2022 •

edited

Loading

pokey commented Jun 17, 2022

purpleP commented Jun 17, 2022

auscompgeek commented Jun 17, 2022 •

edited

Loading

auscompgeek commented Jun 17, 2022

pokey commented Jun 17, 2022

pokey commented Jul 5, 2022

Adding Rust Support #546

Adding Rust Support #546

Conversation

trace-andreason commented Feb 4, 2022 • edited by pokey Loading

What

Checklist

trace-andreason commented Feb 19, 2022

pokey left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pokey Feb 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pokey Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pokey Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

pokey commented May 30, 2022

purpleP commented Jun 11, 2022 • edited Loading

pokey commented Jun 11, 2022

purpleP commented Jun 12, 2022 • edited Loading

pokey commented Jun 12, 2022

purpleP commented Jun 12, 2022

pokey commented Jun 12, 2022

auscompgeek commented Jun 12, 2022

purpleP commented Jun 12, 2022

purpleP commented Jun 14, 2022 • edited Loading

pokey commented Jun 14, 2022

purpleP commented Jun 16, 2022 • edited Loading

pokey commented Jun 17, 2022

purpleP commented Jun 17, 2022

auscompgeek commented Jun 17, 2022 • edited Loading

auscompgeek commented Jun 17, 2022

pokey commented Jun 17, 2022

pokey commented Jul 5, 2022

trace-andreason commented Feb 4, 2022 •

edited by pokey

Loading

pokey left a comment •

edited

Loading

pokey Feb 22, 2022 •

edited

Loading

pokey Mar 18, 2022 •

edited

Loading

pokey Mar 18, 2022 •

edited

Loading

purpleP commented Jun 11, 2022 •

edited

Loading

purpleP commented Jun 12, 2022 •

edited

Loading

purpleP commented Jun 14, 2022 •

edited

Loading

purpleP commented Jun 16, 2022 •

edited

Loading

auscompgeek commented Jun 17, 2022 •

edited

Loading