-
-
Notifications
You must be signed in to change notification settings - Fork 84
Support "instance"
scope type
#1490
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
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 tasks
Superseded by #1497 |
pokey
added a commit
that referenced
this pull request
May 26, 2023
- Implements the "instance" pseudo-scope. It functions grammatical kind of like a scope, but hoists over ranges and uses its input in its own way. - Fixes #29 - Depends on #1496 - See #1490 for some discussion of how we got to this implementation ## Checklist - [x] Handle word scopes - [x] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [x] Merge with #1496 ? - [x] check performance on gigantic file - [x] Come up with story around repeater noises. Maybe interact with some new knausj quick actions interface, where "instance" establishes that the "forward" quick action will perform action on additional target - [x] Document how to add one more selection somewhere: "take that and next instance that" - [x] Capture writing from #1490 - [x] "first three instances" - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [x] Handle narrowing range by selection - [x] I have not broken the cheatsheet
cursorless-bot
pushed a commit
that referenced
this pull request
May 26, 2023
- Implements the "instance" pseudo-scope. It functions grammatical kind of like a scope, but hoists over ranges and uses its input in its own way. - Fixes #29 - Depends on #1496 - See #1490 for some discussion of how we got to this implementation ## Checklist - [x] Handle word scopes - [x] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [x] Merge with #1496 ? - [x] check performance on gigantic file - [x] Come up with story around repeater noises. Maybe interact with some new knausj quick actions interface, where "instance" establishes that the "forward" quick action will perform action on additional target - [x] Document how to add one more selection somewhere: "take that and next instance that" - [x] Capture writing from #1490 - [x] "first three instances" - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [x] Handle narrowing range by selection - [x] I have not broken the cheatsheet
thetomcraig-aya
pushed a commit
to thetomcraig/cursorless
that referenced
this pull request
Mar 27, 2024
- Implements the "instance" pseudo-scope. It functions grammatical kind of like a scope, but hoists over ranges and uses its input in its own way. - Fixes cursorless-dev#29 - Depends on cursorless-dev#1496 - See cursorless-dev#1490 for some discussion of how we got to this implementation ## Checklist - [x] Handle word scopes - [x] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [x] Merge with cursorless-dev#1496 ? - [x] check performance on gigantic file - [x] Come up with story around repeater noises. Maybe interact with some new knausj quick actions interface, where "instance" establishes that the "forward" quick action will perform action on additional target - [x] Document how to add one more selection somewhere: "take that and next instance that" - [x] Capture writing from cursorless-dev#1490 - [x] "first three instances" - [x] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [x] Handle narrowing range by selection - [x] I have not broken the cheatsheet
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is some real ambiguity here. The sub-phrase
"instance air past bat"
could be interpreted three ways:"air past bat"
, eg if you had a function with the expressiona + b
appearing in multiple places, you might want to select all instances ofa + b
in the function with"from funk take every instance air past bat"
"air"
within the region from the cursor through"bat"
. For example, if you had the tokenapartment
appearing multiple times, you might want to select every instance of that token between your cursor and the tokenbanana
by saying"take every instance air past bat"
"air"
and extends through"bat"
. For example, you might be able to see theapartment
token, and want to create a range that extends from the previous instance of that token past the tokenbanana
by saying"take previous instance air past bat"
. This case is especially salient when you consider the following pair:"take this past next instance this"
andtake previous instance this past this"
. The former feels quite useful, especially if shortened to something like"take past next instance"
, and combined with multiple cursors: you could use this if you have a bunch cursors each sitting within a different token and want each to select past the next instance of that token. The latter feels like it should also be useful, but causes precedence conflicts with meaning from 1) aboveIn addition, both approaches I've tried result in some fairly complex rewrite rules to handle the grammatical precedence rules. I think maybe we were onto the right approach with the
"from"
action, but applying it in the wrong place. Maybe instead of using"from"
to prepare the pipeline stages leading up to the input to our modifier, we really want to prepare the pipeline stages that generate the"instance"
scope type itself. We could restrict"instance"
to always "eat" the next modifier or mark, similar to what we do with"tail"
today. We could have an optimisation where if the"instance"
bookmark is set, then it doesn't eat the next modifier or mark. Then the above would be (assuming we use"with"
as the bookmark action:"with air past bat take every instance funk"
"take every instance air past bat"
(this one just works like we want because"instance"
eats"air"
and nothing else). Note that it's more awkward if we want instances of"first word air"
starting from cursor through"bat"
. In that case it would try to just eat"first word"
and search for instances of the first word of the current token within the range"air past bat"
. To avoid that you'd say"with first word air take every instance past bat"
. I wonder if the fact that you need a totally different grammar for situations where you want something more than just a mark will trip people up. We could alternatively make it so that by default it eats everything until it hits a mark within a given primitive target.For the original motivating case
"take every instance two tokens air from inside"
, the modification that"take every instance air past bat from funk"
"take every instance air from past bat"
"take ???"
Combine
"with"
and"from"
. Would want to make sure these actions don't reset / use each other's marks somehow"with air past bat from funk take every instance"
(alternatively just use a cursor:"take air past bat from funk take every instance"
)"from past bat take every instance air"
/"from past bat take every instance first word air"
"take previous instance air past bat"
/"take past next instance"
Checklist
[in]
/[from]
to the instance scope type itself? It is intuitive to say"every <scope in ..."
so might not be a bad idea to just support it everywhere"take next instance"
with multiple cursors"take past next instance"
with multiple cursors"take past next instance"
and"take previous instance past this"
. Due to the proposed precedence rules, the former would select from cursor past the next instance of the token under the cursor, for every cursor, but the latter would look for the previous instance of the cursor past "this", which is nonsense