Skip to content

Migrate python scope "value" to new scope handler #1877

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

Merged
merged 13 commits into from
Sep 10, 2023
64 changes: 64 additions & 0 deletions data/playground/python/values.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Useful commands:
# "visualize value"
# "visualize value iteration"
# "take value"
# "chuck value"
# "chuck every value"

# the argument value "True" that is part of "b=True"
# is "value" scope
def func(b=True, c=True):
# the returned value "1" that is part of "return 1"
# is "value" scope
if b is True:
return 1
else:
return 0

# the argument value "False" that is part of "b=False"
# is "value" scope
func(b=False)
# but here, there is no "value" scope for "False" since no argument
func(False)

# both "1.5" and "25" are "value" scope
val = 1.5
val /= 25

val1, val2 = 0, 5
val1 = 0, val2 = 5
val1 = val2

def my_funk(value: str) -> str:
print(value)

def my_funk(value: str = "hello", other: bool=False) -> str:
print(value)

# we can say "change every value" to allow modifying all the values in one go
def foo():
a = 0
b = 1
c = 2

# But we don't support outside of a function yet
a = 0
b = 1
c = 2

# values of a Python "dictionary" are "value" scope
# we can say "chuck every value" to convert the dict into a set
d1 = {"a": 1, "b": 2, "c": 3}

_ = {value: key for (key, value) in d1.items()}

# complex ones
_ = {{"a": 1, "b": 2, "c": 3}: 1, d1: 2}
_ = {{1, 2, 3}: 1, {2, 3, 4}: 2}

# we don't want the access to a a Python "dictionary"
# value to be of "value" scope so we have it here
# to be sure we ignore it
d1["a"]

value = "hello world"
22 changes: 0 additions & 22 deletions packages/cursorless-engine/src/languages/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,28 +80,6 @@ const nodeMatchers: Partial<
"parameters.identifier!",
"*[name]",
],
value: cascadingMatcher(
leadingMatcher(
["assignment[right]", "augmented_assignment[right]", "~subscript[value]"],
[
":",
"=",
"+=",
"-=",
"*=",
"/=",
"%=",
"//=",
"**=",
"&=",
"|=",
"^=",
"<<=",
">>=",
],
),
patternMatcher("return_statement.~return!"),
),
argumentOrParameter: cascadingMatcher(
argumentMatcher("parameters", "argument_list"),
matcher(patternFinder("call.generator_expression!"), childRangeSelector()),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
languageId: python
command:
version: 6
spokenForm: change value
action:
name: clearAndSetSelection
target:
type: primitive
modifiers:
- type: containingScope
scopeType: {type: value}
usePrePhraseSnapshot: true
initialState:
documentContents: |-
if (x := 0) < 1:
pass
selections:
- anchor: {line: 0, character: 4}
active: {line: 0, character: 4}
marks: {}
finalState:
documentContents: |-
if (x := ) < 1:
pass
selections:
- anchor: {line: 0, character: 9}
active: {line: 0, character: 9}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
languageId: python
command:
version: 6
spokenForm: change value
action:
name: clearAndSetSelection
target:
type: primitive
modifiers:
- type: containingScope
scopeType: {type: value}
usePrePhraseSnapshot: true
initialState:
documentContents: |-
match aaa:
case {"bbb": ccc}:
pass
selections:
- anchor: {line: 1, character: 10}
active: {line: 1, character: 10}
marks: {}
finalState:
documentContents: |-
match aaa:
case {"bbb": }:
pass
selections:
- anchor: {line: 1, character: 17}
active: {line: 1, character: 17}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
languageId: python
command:
version: 6
spokenForm: chuck value
action:
name: remove
target:
type: primitive
modifiers:
- type: containingScope
scopeType: {type: value}
usePrePhraseSnapshot: true
initialState:
documentContents: |-
if (x := 0) < 1:
pass
selections:
- anchor: {line: 0, character: 4}
active: {line: 0, character: 4}
marks: {}
finalState:
documentContents: |-
if (x) < 1:
pass
selections:
- anchor: {line: 0, character: 4}
active: {line: 0, character: 4}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
languageId: python
command:
version: 6
spokenForm: chuck value
action:
name: remove
target:
type: primitive
modifiers:
- type: containingScope
scopeType: {type: value}
usePrePhraseSnapshot: true
initialState:
documentContents: |-
match aaa:
case {"bbb": ccc}:
pass
selections:
- anchor: {line: 1, character: 10}
active: {line: 1, character: 10}
marks: {}
finalState:
documentContents: |-
match aaa:
case {"bbb"}:
pass
selections:
- anchor: {line: 1, character: 10}
active: {line: 1, character: 10}
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
[tool.black]
target-version = ["py39"]
force-exclude = ["vendor", "data/playground"]

[tool.ruff]
select = ["E", "F", "C4", "I001", "UP", "SIM"]
ignore = ["E501", "SIM105", "UP035"]
target-version = "py39"
extend-exclude = ["vendor"]
extend-exclude = ["vendor", "data/playground/**/*.py"]
76 changes: 74 additions & 2 deletions queries/python.scm
Copy link
Member

Choose a reason for hiding this comment

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

It looks like you've lost a bunch of value captures by making the types more specific here. Search in tree-sitter-python for 'value'. Eg we lost values within match statement patterns, values in named expressions, etc. I'd be inclined to keep a generic value: (_) of some sort as that is often what we want

Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,58 @@
(with_statement)
] @statement

;;!! a = 25
;;! ^^
;;! xxxxx
;;! ------
(assignment
(_) @_.leading.start.endOf
.
right: (_) @value @_.leading.end.startOf
) @_.domain
Copy link
Member

@pokey pokey Sep 10, 2023

Choose a reason for hiding this comment

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

fwiw I do think this pattern and the following one (augmented_assignment) are potentially a compelling use case for #has-type? predicate, as that would allow them to be merged into one pattern. @AndreasArvidsson @josharian wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To me it is the same as dictionary vs dictionary_comprehension as discussed in 36c2f4c

Copy link
Member

@pokey pokey Sep 10, 2023

Choose a reason for hiding this comment

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

Agreed it is quite similar. In that case we managed to avoid duplication by rewriting the pattern another way, but I don't see a way out of it here other than has-type


;;!! a /= 25
;;! ^^
;;! xxxxxx
;;! -------
(augmented_assignment
(_) @_.leading.start.endOf
.
right: (_) @value @_.leading.end.startOf
) @_.domain

;;!! d = {"a": 1234}
;;! ^^^^
;;! xxxxxx
;;! ---------
;;!! {value: key for (key, value) in d1.items()}
;;! ^^^
;;! xxxxx
;;! ----------
;;!! def func(value: str = ""):
;;! ^^
;;! xxxxx
;;! ---------------
(
(_
(_) @_.leading.start.endOf
.
value: (_) @value @_.leading.end.startOf
) @_.domain
(#not-type? @_.domain subscript)
)

;;!! return 1
;;! ^
;;! xx
;;! --------
;;
;; NOTE: in tree-sitter, both "return" and the "1" are children of `return_statement`
;; but "return" is anonymous whereas "1" is named node, so no need to exclude explicitly
Comment on lines +76 to +77
Copy link
Member

Choose a reason for hiding this comment

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

This comment might be overkill. We leverage this kind of assumption all over the place. For example, I'm leveraging this kind of assumption in my leading matchers above, where I rely on the fact that = is anonymous whereas the name / type before it is not

(return_statement
(_) @value
) @_.domain

(comment) @comment @textFragment

(string
Expand Down Expand Up @@ -77,6 +129,26 @@
(module) @statement.iteration
(module) @namedFunction.iteration @functionName.iteration
(class_definition) @namedFunction.iteration @functionName.iteration
(_
body: (_) @statement.iteration

;;!! def foo():
;;!! a = 0
;;! <*****
;;!! b = 1
;;! *****
;;!! c = 2
;;! *****>
(block) @statement.iteration @value.iteration

;;!! {"a": 1, "b": 2, "c": 3}
;;! **********************
(dictionary
"{" @value.iteration.start.endOf
"}" @value.iteration.end.startOf
)

;;!! def func(a=0, b=1):
;;! ********
(parameters
"(" @value.iteration.start.endOf
")" @value.iteration.end.startOf
)