Skip to content

Automatically try to remove scopes as if they're token, line, block, file if they have no removal behaviour #855

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

Closed
pokey opened this issue Jul 11, 2022 · 9 comments · Fixed by #2783
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@pokey
Copy link
Member

pokey commented Jul 11, 2022

Most of our tree-sitter scopes don't remove newlines, and some of them don't remove spaces. We'd like a way to make them all automatically remove the appropriate whitespace

I would argue that we should detect if something stops at a line boundary and use newline delimiter if so.

Implementation

Where

Let's start by putting it on SyntacticScopeTarget

What

If target has leading or trailing range, we use today's behaviour.

Otherwise, we see if the "core removal range" corresponds to the entire file, then falling back to block, then line, then token. If it corresponds to one of these, we forward calls to removalRange to the upgraded target

The core removal range will be the value passed in to removalRange in constructor, otherwise content range.

Other thoughts

Alternately we could add this implementation to BaseTarget, so that things like SyntacticScopeTarget will get it, but not get LineTarget, TokenTarget, UntypedTarget (?) will override it, but let's start with SyntacticScopeTarget for now

@pokey pokey added the enhancement New feature or request label Jul 11, 2022
@AndreasArvidsson
Copy link
Member

Do you want this behavior only for pour or also for bring? Should some scopes types be omitted? Because now when we have textual item scope "pour item" should definitely not insert a line just because it is the last token on the line.

@pokey
Copy link
Member Author

pokey commented Jul 11, 2022

If something is the last token on the line it will not count as a line. It needs to start and end on line boundary

It will also only be if the scope type has no explicit delimiter set, so wouldn't affect item for that reason as well

@AndreasArvidsson
Copy link
Member

AndreasArvidsson commented Jul 11, 2022

I guess for scope types that have no specific insertion delimiter this could work. How will you handle multiline targets? Check both first and last line? Leading and trailing white space will be omitted from the line calculation?

@pokey
Copy link
Member Author

pokey commented Jul 11, 2022

Yep!

@pokey
Copy link
Member Author

pokey commented Jul 11, 2022

I think we can also consider detecting we're a block and set it to \n\n, though not quite as critical as the newline one. That one gets me because it feels like a regression in situations like this one

@pokey
Copy link
Member Author

pokey commented Jul 18, 2022

This one is also super annoying for "section" in markdown, eg "clone up section"

@pokey pokey added this to the 0.27.0 milestone Jul 18, 2022
@AndreasArvidsson
Copy link
Member

The current solution for these is of course that they have the wrong delimiter. Section should definitely not insert or remove as a token.

@pokey
Copy link
Member Author

pokey commented Jul 19, 2022

Yeah maybe for now let's just add them to that switch statement, but I do worry it's going to start feeling like a game of whack-a-mole

@AndreasArvidsson
Copy link
Member

Agreed. We should definitely have a proper look at an alternative solution. But just for now I think we should just add them to the delimiter list.

@pokey pokey changed the title Automatically detect delimiter for scope type targets Automatically try to upgrade scopes to token, line, block, file if they have no removal behaviour Oct 23, 2022
@pokey pokey changed the title Automatically try to upgrade scopes to token, line, block, file if they have no removal behaviour Automatically try to remove scopes as if they're token, line, block, file if they have no removal behaviour Oct 23, 2022
@pokey pokey modified the milestones: Short list, Long list Jul 6, 2023
@pokey pokey mentioned this issue Jul 7, 2023
3 tasks
@AndreasArvidsson AndreasArvidsson self-assigned this Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants