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

feat: Highlight exit points of async blocks #18152

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

CryZe
Copy link
Contributor

@CryZe CryZe commented Sep 19, 2024

Async blocks act similar to async functions in that the await keywords are related, but also act like functions where the exit points are related.

Fixes #18147

Async blocks act similar to async functions in that the await keywords
are related, but also act like functions where the exit points are
related.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2024
@@ -281,99 +281,95 @@ fn highlight_references(
}
}

// If `file_id` is None,
pub(crate) fn highlight_exit_points(
fn hl_exit_points(
Copy link
Contributor Author

@CryZe CryZe Sep 19, 2024

Choose a reason for hiding this comment

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

It looks like a ton of stuff happened here, but nothing is actually changed and the function just moved out, so it can be called from highlight_yield_points. You can hide whitespace changes on the cog symbol above to make this easier on the eyes xD

@CryZe
Copy link
Contributor Author

CryZe commented Sep 19, 2024

Technically an async block is also a closure with captures, but I didn't quite feel comfortable enough to make that change (yet?) as my first meaningful contribution.

@Veykril
Copy link
Member

Veykril commented Sep 24, 2024

Technically an async block is also a closure with captures, but I didn't quite feel comfortable enough to make that change (yet?) as my first meaningful contribution.

We dont compute captures for async functions yet and that is no simple task (likely resolved by #16173 once thats finished)

Comment on lines 520 to 523
// Async blocks act similar to closures. So we want to
// highlight their exit points too.
let exit_points = hl_exit_points(sema, block_expr.async_token(), block_expr.clone().into());
merge_map(&mut res, exit_points);
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, so the thing is, when the cursor is on async we only want to render the yield points, not exit points (this is useful for functions where you can get either depending on whether you put the cursor on the async or fn keyword). For async blocks you don't really have a simple way to get the exit points though so having it render both on async does make sense imo (but not when the cursor is on an `await).

So I would want to branch this by block_expr.async_token() == token to only trigger this if the cursor is on the async token of the block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, it took a bit, but it should be good now.

Copy link
Member

Choose a reason for hiding this comment

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

No worries

@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 25, 2024
This ensures that when being on an `await` token, it still only
highlights the yield points and not the exit points.
@Veykril
Copy link
Member

Veykril commented Oct 14, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Oct 14, 2024

📌 Commit 68c263b has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 14, 2024

⌛ Testing commit 68c263b with merge 4717b2b...

@bors
Copy link
Contributor

bors commented Oct 14, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 4717b2b to master...

@bors bors merged commit 4717b2b into rust-lang:master Oct 14, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Question Mark Operator in Control Flow Highlighting
4 participants