Skip to content

Update scope type modifier to expand on both ends #484

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
1 of 4 tasks
AndreasArvidsson opened this issue Jan 15, 2022 · 2 comments · Fixed by #1031
Closed
1 of 4 tasks

Update scope type modifier to expand on both ends #484

AndreasArvidsson opened this issue Jan 15, 2022 · 2 comments · Fixed by #1031
Labels
enhancement New feature or request

Comments

@AndreasArvidsson
Copy link
Member

AndreasArvidsson commented Jan 15, 2022

Right now you can take lines paragraphs and soon non whitespace sequences that expands from both anchor and active of the selection. Currently containing scopes only use start off the selection.

I want both of these to have the same result. (air and bat are in different functions)
take funk air past bat
take air past bat take funk

I was thinking that we could implement this first thing in the selection/scope modifier by basically calling the specific implementation twice, once for both ends. To day we pass the entire selection to the specific modifier(line, funk, ...). What if we pass in a single position instead?

const startSelection = getSelectionForScope(selection.start, "function"); // function is of course a variable
if (selection.isEmpty() 
  || getNodeAtLocation(selection.start).id === getNodeAtLocation(selection.end).id) {
    return startSelection;
}
const endSelection = getSelectionForScope(selection.end, "function");
return  getSelectionFromPositions(startSelection.start, endSelection.end);

edit (@pokey)

Let's implement this one in a new ContainingScope stage that all containing scopes share. The algorithm should be as described in #629 (comment). The initial implementation will be folded into #629

@pokey
Copy link
Member

pokey commented Jan 16, 2022

I don't think this will work for "round", even if we decide to make it consistent with "funk". The problem is if the user has (()), and says "take round repper", referring to the first ). When expanding from the end, it will look right and end up with the whole string, which is incorrect

update: the algorithm described in #629 (comment) should handle this case just fine, because it will first try the start of the target, which is in the center of all the parens, and the returned domain will be (), which weakly contains the end of the target, so it will just return it

@AndreasArvidsson
Copy link
Member Author

Yes some special handling of matching pairs is fine. Just getting all the containing scope and selection types working as one would be nice.

@pokey pokey added the enhancement New feature or request label Jan 16, 2022
This was referenced Jul 4, 2022
pokey added a commit that referenced this issue Apr 19, 2023
- Partially addresses #616
- Partially addresses
#436
- Depends on #1396

## Todo
- [x] **[DISCUSS]** What to do about fallback `iterationScope`? That's
the only thing that is a regression here.
- [x] File issues for FIXMEs
- [x] File issue for defining iteration scopes. Can probably reuse most
of the code from the regular scope handler other than creating the
target
- [x] File issue to add unit tests for scope handlers
- [x] File issue to add some Python scope types where multiple can end
at the same point (due to lack of closing brackets)
- [x] Add test that checks no scope types are duplicated between legacy
and new definition, or file issue to add test
- [x] File PR for my 7783da6 (Add support for domain, leading, trailing,
interior) #1427
- [x] Look through comments on this thread for anything worth filing /
doing
- [x] Open as new PR?
- [x] Remove extraneous test cases
- [x] Double check
#629 (comment);
a lot of those tests we already have for the generic modifier code
- [x] Make sure changes to parse-tree-extension are shipped
- [x] Close #785 if
we fix that
- [x] Comment on #484 saying the process has started and providing link
to example
- [x] Close #797 if
we fix that

---------

Co-authored-by: Pokey Rule <[email protected]>
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