Skip to content

Enable initial r support #2721

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
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

BlueDrink9
Copy link

@BlueDrink9 BlueDrink9 commented Jan 12, 2025

I'm still wrapping my head around treesitter to add the supported scopes, so I'll update this PR and add relevant tests once I do + once the r PR is added to vscode's parse tree

Checklist

@BlueDrink9 BlueDrink9 requested a review from a team as a code owner January 12, 2025 03:42
@AndreasArvidsson AndreasArvidsson marked this pull request as draft January 12, 2025 07:54
@BlueDrink9 BlueDrink9 marked this pull request as ready for review May 13, 2025 21:34
@BlueDrink9
Copy link
Author

I'm marking this as ready to make adding scopes easier. Once r support is included in cursorless, it will be easier for it to pick it up as an option for adding scopes

@AndreasArvidsson
Copy link
Member

Latest version of the parse tree is published and should be available in the marketplace very soon.

[Content] =
[Domain] = 0:0-0:3
>---<
0| abc <- function(arg){
Copy link
Author

Choose a reason for hiding this comment

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

This is the only case that I'm unsure about and is likely to need review, I'm wondering if it would make more sense to include the operator in this for removal and insertion? Although I might struggle a little bit with adding them as custom delimiters

Copy link
Member

Choose a reason for hiding this comment

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

<- should probably be added as a trailing range. That way it will be deleted if you delete the name.
We do the same thing with = in value = 2

@BlueDrink9
Copy link
Author

@AndreasArvidsson ready to review! Thank you again for your help yesterday

@AndreasArvidsson
Copy link
Member

Please add a scope test for an empty argument list as well. The difference is the insertion delimiter. Please have a look at how it's implemented in other languages.

@AndreasArvidsson
Copy link
Member

I would recommend that you create multiple scope files for the same facet instead of adding multiple different code examples in the same file. It's much simpler to review it that way.

[Content] =
[Domain] = 0:0-0:3
>---<
0| abc <- function(arg){
Copy link
Member

Choose a reason for hiding this comment

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

<- should probably be added as a trailing range. That way it will be deleted if you delete the name.
We do the same thing with = in value = 2


[#1 Trailing delimiter] = 0:5-0:6
>-<
0| hello <- "world"
Copy link
Member

Choose a reason for hiding this comment

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

<- should be part off the trailing range. Have a look at how we do this in other languages.


[#2 Trailing delimiter] = 1:5-1:6
>-<
1| hello = "world"
Copy link
Member

Choose a reason for hiding this comment

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

= should be part of the trailing range


[#1 Trailing delimiter] = 0:5-0:6
>-<
0| hello <- "world"
Copy link
Member

Choose a reason for hiding this comment

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

<- should be part of the trailing range


[#2 Trailing delimiter] = 1:5-1:6
>-<
1| hello = "world"
Copy link
Member

Choose a reason for hiding this comment

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

= should be part of the trailing range


[#2 Leading delimiter] = 1:7-1:8
>-<
1| hello = "world"
Copy link
Member

Choose a reason for hiding this comment

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

= should be part of the leading range


[#1 Leading delimiter] = 0:8-0:9
>-<
0| hello <- "world"
Copy link
Member

Choose a reason for hiding this comment

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

<- should be part of the leading range

[#2 Content] =
[#2 Domain] = 1:8-1:15
>-------<
1| hello = "world"
Copy link
Member

Choose a reason for hiding this comment

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

= should be part of the leading range

@BlueDrink9 BlueDrink9 force-pushed the main branch 2 times, most recently from 8bb8dee to 3bf717c Compare May 17, 2025 03:27
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.

2 participants