Skip to content

feat: Highlight exit and yield points #9375

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

Merged
merged 11 commits into from
Jun 24, 2021
Merged

feat: Highlight exit and yield points #9375

merged 11 commits into from
Jun 24, 2021

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Jun 22, 2021

hvx3IWepc5
Code_YyMhqES0LX

Fixes #4691

@matklad
Copy link
Member

matklad commented Jun 22, 2021

Moves in the right direction! Couple of relevant thoughts:

  • a related functionality is highlighting all exit points of the function, but we probably want a separate request for that (and support in client)
  • document highlights is less than ideal name here. Perhaps just highlight_references?

@kjeremy
Copy link
Contributor

kjeremy commented Jun 22, 2021

* a related functionality is highlighting all exit points of the function, but we probably want a separate request for that (and support in client)

I don't think it needs to be a separate request See microsoft/language-server-protocol#1047

@Veykril
Copy link
Member Author

Veykril commented Jun 23, 2021

Ye I think we don't need a separate request for that, but then the question is how we highlight exit points in regards to the trailing expression as that can overlap with the general ident highlight.

@matklad
Copy link
Member

matklad commented Jun 23, 2021

If the cursor is on ? / return or return type (-> Ty) we should highlight exit points, including expressions in the tail position.

if the cursor is on async / await, we should highlight all yield points.

if the cursor is within trailing expression, we should highlight references or nothing.

The name for all three I think is “highlight_related”?

@Veykril Veykril marked this pull request as ready for review June 23, 2021 14:49
@Veykril
Copy link
Member Author

Veykril commented Jun 23, 2021

Yield point and exit point highlighting is implemented as well now. Gotta fix(and probably find) a few more special cases as well as do some minor cleanup still.

Also never type expressions being highlighted as exit points is missing still like intellij does, shouldnt be too hard to add as well I believe.

@Veykril Veykril changed the title Move document highlighting computation from rust-analyzer to ide feat: Highlight exit and yield points Jun 23, 2021
@Veykril
Copy link
Member Author

Veykril commented Jun 23, 2021

Okay things are working but VSCode doesn't send a highlight request when the cursor goes onto non identifier symbols from what I can tell, that means clicking on ? and -> doesn't send a request currently. I suppose this is a bug on VSCode's side as I don't see anything in this regard in the lsp docs. Opened microsoft/vscode#127007

@Veykril
Copy link
Member Author

Veykril commented Jun 23, 2021

Currently when the trailing expression is an if or match expression the entire expression gets highlighted as an exit point. Now I wonder if it would make more sense to only highlight the arm/branch expressions in this case? Seems more elegant to me.

@matklad
Copy link
Member

matklad commented Jun 23, 2021

Yeah, it’s better to highlight expressions in the tail position. I think we should have functionality for that implemented somewhere already, but I don’t remember where. Ok-wrapping maybe?

@matklad
Copy link
Member

matklad commented Jun 23, 2021

Got it! We have “change return type to Result” assist!

@Veykril
Copy link
Member Author

Veykril commented Jun 23, 2021

Oh ye looks like that assist does similar things to what is wanted here, I'll take a look at that tomorrow

Comment on lines +569 to +575
8 => 'a: loop {
'b: loop {
break 'a 5;
// ^^^^^^^^^^
break 'b 5;
break 5;
};
Copy link
Member Author

@Veykril Veykril Jun 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if I overdid this a bit by also considering (labeled) breaks in loops 😄

@Veykril
Copy link
Member Author

Veykril commented Jun 24, 2021

Managed to simplify things a bit more, but I couldn't really reuse the wrap_return_type_in_result logic for tails fully as its a bit different than whats needed here. Might be able to merge the logic somehow still but I'll take a look at that another time.
bors r+

@bors
Copy link
Contributor

bors bot commented Jun 24, 2021

@bors bors bot merged commit 264716e into rust-lang:master Jun 24, 2021
@Veykril Veykril deleted the hldoc branch June 24, 2021 15:59
bors bot added a commit that referenced this pull request Jun 26, 2021
9413: internal: Deduplicate ast expression walking logic r=Veykril a=Veykril

Deduplicates the duplication introduced in #9375 and #9396 while also fixing a few bugs in both the assist and related highlighting.

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
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

Successfully merging this pull request may close these issues.

Semantic highlighting for function exit points
3 participants