Skip to content
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.

Sign up for GitHub

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

Update python highlighting to take advantage of themes. #2451

Merged
merged 13 commits into from
May 20, 2022
Merged

Update python highlighting to take advantage of themes. #2451

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
62a6345
Change python highlights.scm to more fully utilize the themes.
Zeddicus414 May 10, 2022
1e4a9da
dark_plus theme constructor should be green.
Zeddicus414 May 10, 2022
25815df
Merge branch 'helix-editor:master' into master
Zeddicus414 May 12, 2022
d5b6bc4
Python highlight improvements: type, parameter etc
May 13, 2022
fc84edd
Merge pull request #1 from paul-scott/types
Zeddicus414 May 13, 2022
9da79d9
Python handling highlighting parameters with defaults
May 14, 2022
ef740fb
Python highlight keyword argument as parameter
May 14, 2022
065ab2a
Merge pull request #2 from paul-scott/types
paul-scott May 14, 2022
41cfaed
Python highlight decorator attribute
May 14, 2022
d3ee550
Merge pull request #3 from paul-scott/types
paul-scott May 14, 2022
2009d2b
Merge branch 'helix-editor:master' into master
Zeddicus414 May 15, 2022
b07f84f
Merge branch 'helix-editor:master' into master
Zeddicus414 May 16, 2022
6756086
Merge branch 'helix-editor:master' into master
Zeddicus414 May 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 33 additions & 18 deletions runtime/queries/python/highlights.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
; Identifier naming conventions

((identifier) @constant
(#match? @constant "^[A-Z_]*$"))

((identifier) @constructor
(#match? @constructor "^[A-Z]"))

((identifier) @constant
(#match? @constant "^[A-Z][A-Z_]*$"))
; Types

((identifier) @type
(#match?
@type
"^(bool|bytes|dict|float|frozenset|int|list|set|str|tuple)$"))

(type (identifier)) @type

; Builtin functions

((call
function: (identifier) @function.builtin)
(#match?
@function.builtin
"^(abs|all|any|ascii|bin|bool|breakpoint|bytearray|bytes|callable|chr|classmethod|compile|complex|delattr|dict|dir|divmod|enumerate|eval|exec|filter|float|format|frozenset|getattr|globals|hasattr|hash|help|hex|id|input|int|isinstance|issubclass|iter|len|list|locals|map|max|memoryview|min|next|object|oct|open|ord|pow|print|property|range|repr|reversed|round|set|setattr|slice|sorted|staticmethod|str|sum|super|tuple|type|vars|zip|__import__)$"))
Copy link
Member

Choose a reason for hiding this comment

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

These are built in functions though (if used with parentheses) https://www.programiz.com/python-programming/methods/built-in/str

VSCode highlights these differently, but I'd argue our highlighting is more precise here since we can distinguish between a type and a built-in function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is true, however, I would respectfully argue that the type changing properties of these functions and their uses for type checking tools like Mypy and Typeguard more closely relate these particular functions to types.

At least in the codebases I am using, these functions are increasingly called to clarify types for type checkers as much as to change types at runtime.

Copy link
Contributor

@paul-scott paul-scott May 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Independently of this (only just found this PR now), I implement some changes to the python highlighting. This touches on type highlighting among other things (parameters, decorators, etc.). I managed to treat str and the like as @function.builtin when being used as a function, and treat them as @type when being used in type hints. See #2457

Copy link
Contributor

Choose a reason for hiding this comment

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

@Zeddicus414 if you think this is ready, go ahead and mark this PR as open again.

We've merged in the highlights work in #2457. archseer one of the changes keeps str, int, list, and so on as @function.builtin for the cases where they are being called, otherwise they are treated as @type.builtin. So hopefully that addressed your concern. Hopefully the sequence of commits is not too messy to follow, otherwise let us know if we need to clean things up.

"^(abs|all|any|ascii|bin|breakpoint|bytearray|callable|chr|classmethod|compile|complex|delattr|dir|divmod|enumerate|eval|exec|filter|format|getattr|globals|hasattr|hash|help|hex|id|input|isinstance|issubclass|iter|len|locals|map|max|memoryview|min|next|object|oct|open|ord|pow|print|property|range|repr|reversed|round|setattr|slice|sorted|staticmethod|sum|super|type|vars|zip|__import__)$"))

; Function calls

Expand All @@ -30,7 +39,6 @@

(identifier) @variable
(attribute attribute: (identifier) @variable.other.member)
(type (identifier) @type)

; Literals

Expand Down Expand Up @@ -81,41 +89,48 @@
">>"
"|"
"~"
"and"
"in"
"is"
"not"
"or"
] @operator

[
"as"
"assert"
"async"
"await"
"break"
"class"
"continue"
"def"
"del"
"elif"
"else"
"except"
"exec"
"finally"
"for"
"from"
"global"
"if"
"import"
"lambda"
"nonlocal"
"pass"
"print"
"raise"
"return"
"try"
"while"
"with"
"yield"
] @keyword.control

(for_statement "in" @keyword.control)
(for_in_clause "in" @keyword.control)

[
"and"
"async"
"class"
"def"
"del"
"exec"
"global"
"in"
"is"
"lambda"
"nonlocal"
"not"
"or"
"print"
] @keyword

2 changes: 1 addition & 1 deletion runtime/themes/dark_plus.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"type" = { fg = "type" }
"type.builtin" = { fg = "type" }
"type.enum.variant" = { fg = "constant" }
"constructor" = { fg = "constant" }
"constructor" = { fg = "type" }
Copy link
Member

Choose a reason for hiding this comment

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

\cc @kirawi to confirm this

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I think that the usage of @constructor has become somewhat conflated with @type as in many highlight queries it applies to all identifiers with ^[A-Z]. I'm fine with this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

The recent commits we made to this PR fixed up the usage of @constructor so it is no longer conflated with @type in python at least. I suspect originally that was the main motivation for @Zeddicus414 wanting to change this. For style reasons, maybe the change here is still a good idea. I just thought I'd point this out.

"variable.other.member" = { fg = "variable" }

"keyword" = { fg = "blue2" }
Expand Down