Add support for 'all' modifier for items within a collection #597

Will-Sommers · 2022-03-09T21:42:20Z

What

This PR addresses Issue 473 and adds support for the "all" modifier within a list.

I'm pretty rusty on the .ts types side and also might be missing some context about casting Range as Selection and setting it. I think there is likely a more elegant way.

Checklist

I have added tests
Update cheatsheet/docs

Will-Sommers · 2022-03-09T21:50:51Z

Fixing/Running tests locally, still needing to close VSCode.

Edit:
Running the most recent commit, passing on GH, is failing for me locally as well. Not a huge deal but I noticed it failing in a similar way on another branch. I'll make another issue but it looks like indentation is off for some(~24) tests, mostly around `"(if | state) wrap this" and another indentation one. Mostly in Python but also Java and TypeScript.

Here's an example:

  22) recorded test cases
       /Users/.../recorded/languages/typescript/ifElseWrapThis:

      AssertionError [ERR_ASSERTION]: Unexpected final state
      + expected - actual

       {
      -  "documentContents": "if () {\n  const foo = \"hello\";\n} else {\n  \n}"
      +  "documentContents": "if () {\n    const foo = \"hello\";\n} else {\n    \n}"
         "selections": [
           {
             "active": {
               "character": 4

…Sommers/cursorless-vscode into issue-473-add-all-modifier

Will-Sommers · 2022-04-15T03:02:01Z

cursorless-talon/src/modifiers/containing_scope.py

-            "type": "containingScope",
-            "scopeType": m.cursorless_scope_type,
-            "includeSiblings": m[0] == "every",
+    """Expand to every scope"""


Definitely open to putting this into another file, my python is a little weak as is my understanding of how it interacts with Talon. Just lmk.

Will-Sommers · 2022-04-15T03:02:16Z

cursorless-talon/src/modifiers/containing_scope.py


-@mod.capture(rule="[every] {user.cursorless_scope_type}")
+@mod.capture(rule="[(every|all)] {user.cursorless_scope_type}")


Would be nifty if we could put this into a capture against the set above.

You could use a Talon list, no?

Will-Sommers · 2022-04-15T14:49:29Z

cursorless-talon/src/modifiers/containing_scope.py

+            "modifier": {
+                "type": "everyScope",
+                "scopeType": m.cursorless_scope_type,
+                "contiguousRange": m[0] == "all",


note: re-add check on for items here

Will-Sommers · 2022-04-18T18:11:38Z

Looking at the test failures. A few notes to myself:

Re-running the tests via the GH UI resulted in Error: Cannot activate the 'Cursorless' extension because it depends on unknown extension 'pokey.parse-tree'
Tests are failing initially because this PR changes change the type from containingScope to everyScope

Trying to get the test transfomer running as well, will need to take a look later.

pokey

Looks like this one's going to need some conflict resolution / tweaking to account for changes introduced in #672. I also appear to have some pending comments; not sure how relevant they still har

pokey · 2022-05-13T09:52:35Z

cursorless-talon/src/modifiers/containing_scope.py

+mod.list("select_multiple_modifiers", desc="modifiers for multiple selections")
+multiple_modifiers = {"every", "all"}
+ctx.lists["user.select_multiple_modifiers"] = multiple_modifiers


I'd argue that these terms should be csv-configurable

pokey · 2022-05-13T09:54:00Z

cursorless-talon/src/modifiers/containing_scope.py

+            "modifier": {
+                "type": "everyScope",
+                "scopeType": m.cursorless_scope_type,
+                "contiguousRange": m[0] == "all" and m[1] == "collectionItem",


Why do we only support this one for collectionItem?

pokey · 2022-08-17T13:00:38Z

I'm going to close this one for now, because:

for "chuck", we can just use "chuck every"
For other use cases, I think I'd lean towards implementing this one using Add "range" modifier #524
This PR has lots of merge conflicts and seems like it will be painful to take to the finish line

Feel free to re-open if you wanna take a crack at it

Will-Sommers added 2 commits March 9, 2022 16:36

Add support for 'all' modifier for items

c07b8ad

Add test for map

e1320fa

Will-Sommers requested a review from pokey as a code owner March 9, 2022 21:42

Update function calls to support extra parameter

e1c103b

Will-Sommers mentioned this pull request Mar 9, 2022

Tests failing locally #598

Closed

Will-Sommers added 6 commits March 10, 2022 17:46

Update README, ensure no odd behavior when trying to select 'all key'

cece689

Merge branch 'main' into issue-473-add-all-modifier

7ec8afa

Update PR to reflect notes from Pokey

af830f9

Merge branch 'issue-473-add-all-modifier' of https://github.com/Will-…

8533879

…Sommers/cursorless-vscode into issue-473-add-all-modifier

Fix type signature

540a994

Update tess to reflect new modifier

e94419a

Will-Sommers commented Apr 15, 2022

View reviewed changes

Will-Sommers added 2 commits April 14, 2022 23:04

Fix formatting in .py file

f072856

more python formatting

70e8a52

Will-Sommers commented Apr 15, 2022

View reviewed changes

Will-Sommers added 3 commits April 18, 2022 11:49

Fix formatting, finally get python black working

22b08a8

Use Talon list for multiple scope modifiers

9b319d9

Merge branch 'cursorless-dev:main' into issue-473-add-all-modifier

b19123c

Will-Sommers closed this Apr 18, 2022

Will-Sommers reopened this Apr 18, 2022

pokey reviewed Jun 8, 2022

View reviewed changes

pokey closed this Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for 'all' modifier for items within a collection #597

Add support for 'all' modifier for items within a collection #597

Will-Sommers commented Mar 9, 2022 •

edited

Loading

Will-Sommers commented Mar 9, 2022 •

edited

Loading

Will-Sommers Apr 15, 2022

Will-Sommers Apr 15, 2022

pokey Apr 15, 2022

Will-Sommers Apr 15, 2022

Will-Sommers commented Apr 18, 2022 •

edited

Loading

pokey left a comment

pokey May 13, 2022

pokey May 13, 2022

pokey commented Aug 17, 2022


		@mod.capture(rule="[every] {user.cursorless_scope_type}")
		@mod.capture(rule="[(every\|all)] {user.cursorless_scope_type}")

Add support for 'all' modifier for items within a collection #597

Add support for 'all' modifier for items within a collection #597

Conversation

Will-Sommers commented Mar 9, 2022 • edited Loading

What

Checklist

Will-Sommers commented Mar 9, 2022 • edited Loading

Will-Sommers Apr 15, 2022

Choose a reason for hiding this comment

Will-Sommers Apr 15, 2022

Choose a reason for hiding this comment

pokey Apr 15, 2022

Choose a reason for hiding this comment

Will-Sommers Apr 15, 2022

Choose a reason for hiding this comment

Will-Sommers commented Apr 18, 2022 • edited Loading

pokey left a comment

Choose a reason for hiding this comment

pokey May 13, 2022

Choose a reason for hiding this comment

pokey May 13, 2022

Choose a reason for hiding this comment

pokey commented Aug 17, 2022

Will-Sommers commented Mar 9, 2022 •

edited

Loading

Will-Sommers commented Mar 9, 2022 •

edited

Loading

Will-Sommers commented Apr 18, 2022 •

edited

Loading