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.

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

Some functions are not being colored properly. #11

Closed
nmpsyntax opened this issue Nov 25, 2020 · 17 comments · Fixed by #13 or #27
Closed

Some functions are not being colored properly. #11

nmpsyntax opened this issue Nov 25, 2020 · 17 comments · Fixed by #13 or #27
Assignees
Labels
bug something isn't working good first issue good for new contributors

Comments

@nmpsyntax
Copy link

nmpsyntax commented Nov 25, 2020

nocolor

Some of my functions are not being colored correctly.

morecolor

Here is the code in the images:

<#Tab::     AltTab
<#c::       CenterActiveWindow()
$LAlt::     LAlt()
$*Pause::   Pause()
$AppsKey::  AppsKey()
$CapsLock:: CapsLock()
@mark-wiemer
Copy link
Collaborator

Are you using AHK v2 or v1? The extension is currently written primarily for v1, I am working on improving support for v2, I will look into this over the weekend

@mark-wiemer mark-wiemer self-assigned this Dec 11, 2020
@mark-wiemer mark-wiemer added bug something isn't working good first issue good for new contributors labels Dec 11, 2020
@nmpsyntax
Copy link
Author

Thanks for the response. The version is v1.1.30.03 64-bit Unicode according to the following code:

Msgbox % "v" A_AhkVersion " " (A_PtrSize = 4 ? 32 : 64) "-bit " (A_IsUnicode ? "Unicode" : "ANSI")

@mark-wiemer
Copy link
Collaborator

Thanks for confirming. This will take some looking into. Just to confirm: AltTab will likely remain the blue color because it is not a function call.

@nmpsyntax
Copy link
Author

My initial assumptions might have been wrong regarding the intended colors. Are function calls supposed to be white? I thought they were supposed to be blue because the other functions calls were. Are they blue because keys names are blue and that takes precedence over the fact that they are also function calls? If this is the case, the only issue I see in the coloring of my code is that Pause should be blue because it is a key name and that should take precedence over it also being a function.

@FuPeiJiang
Copy link

in my vscode, function calls are yellow, since these are all function calls (Except for AltTab), I would make them all yellow.


how to do this:
in \vscode-autohotkey-plus-plus\syntaxes\ahk.tmLanguage.json
change the corresponding match of
"name": "keyword.keys.ahk",
to

nested stuff (click to expand) "match": "\\b(?!MsgBox)(?i:new|__New|__Delete|shift|lshift|rshift|alt|lalt|ralt|control|lcontrol|rcontrol|ctrl|lctrl|rctrl|lwin|rwin|appskey|altdown|altup|shiftdown|shiftup|ctrldown|ctrlup|lwindown|lwinup|rwindown|rwinup|lbutton|rbutton|mbutton|wheelup|wheelleft|wheelright|wheeldown|xbutton1|xbutton2|joy1|joy2|joy3|joy4|joy5|joy6|joy7|joy8|joy9|joy10|joy11|joy12|joy13|joy14|joy15|joy16|joy17|joy18|joy19|joy20|joy21|joy22|joy23|joy24|joy25|joy26|joy27|joy28|joy29|joy30|joy31|joy32|joyx|joyy|joyz|joyr|joyu|joyv|joypov|joyname|joybuttons|joyaxes|joyinfo|space|tab|enter|escape|esc|backspace|bs|delete|del|insert|ins|pgup|pgdn|home|end|up|down|left|right|printscreen|ctrlbreak|pause|scrolllock|capslock|numlock|numpad0|numpad1|numpad2|numpad3|numpad4|numpad5|numpad6|numpad7|numpad8|numpad9|numpadmult|numpadadd|numpadsub|numpaddiv|numpaddot|numpaddel|numpadins|numpadclear|numpadup|numpaddown|numpadleft|numpadright|numpadhome|numpadend|numpadpgup|numpadpgdn|numpadenter|f1|f2|f3|f4|f5|f6|f7|f8|f9|f10|f11|f12|f13|f14|f15|f16|f17|f18|f19|f20|f21|f22|f23|f24|browser_back|browser_forward|browser_refresh|browser_stop|browser_search|browser_favorites|browser_home|volume_mute|volume_down|volume_up|media_next|media_prev|media_stop|media_play_pause|launch_mail|launch_media|launch_app1|launch_app2|vk\\d+|sc\\d+)(?!\\()\\b\\b(?!MsgBox)(?i:new|__New|__Delete|shift|lshift|rshift|alt|lalt|ralt|control|lcontrol|rcontrol|ctrl|lctrl|rctrl|lwin|rwin|appskey|altdown|altup|shiftdown|shiftup|ctrldown|ctrlup|lwindown|lwinup|rwindown|rwinup|lbutton|rbutton|mbutton|wheelup|wheelleft|wheelright|wheeldown|xbutton1|xbutton2|joy1|joy2|joy3|joy4|joy5|joy6|joy7|joy8|joy9|joy10|joy11|joy12|joy13|joy14|joy15|joy16|joy17|joy18|joy19|joy20|joy21|joy22|joy23|joy24|joy25|joy26|joy27|joy28|joy29|joy30|joy31|joy32|joyx|joyy|joyz|joyr|joyu|joyv|joypov|joyname|joybuttons|joyaxes|joyinfo|space|tab|enter|escape|esc|backspace|bs|delete|del|insert|ins|pgup|pgdn|home|end|up|down|left|right|printscreen|ctrlbreak|pause|scrolllock|capslock|numlock|numpad0|numpad1|numpad2|numpad3|numpad4|numpad5|numpad6|numpad7|numpad8|numpad9|numpadmult|numpadadd|numpadsub|numpaddiv|numpaddot|numpaddel|numpadins|numpadclear|numpadup|numpaddown|numpadleft|numpadright|numpadhome|numpadend|numpadpgup|numpadpgdn|numpadenter|f1|f2|f3|f4|f5|f6|f7|f8|f9|f10|f11|f12|f13|f14|f15|f16|f17|f18|f19|f20|f21|f22|f23|f24|browser_back|browser_forward|browser_refresh|browser_stop|browser_search|browser_favorites|browser_home|volume_mute|volume_down|volume_up|media_next|media_prev|media_stop|media_play_pause|launch_mail|launch_media|launch_app1|launch_app2|vk\\d+|sc\\d+)(?!\\s*\\()\\b",

note: this will not only match "func()" , but it will also match "func      ()"
change the corresponding match of
"name": "entity.name.function.ahk",
to
"match": "\b(?!MsgBox)([\w]+)(?=\s*\()",


key names take precedence over custom functions
built-in functions take precedence over key names


next time, please paste the code in the images, so someone else won't have to retype it

@FuPeiJiang
Copy link

I'm curious, is there any way to make
"name": "keyword.keys.ahk",
take precedence over
"name": "support.function.ahk",
?


if you want, you can make all functions blue:
"#569cd6"
in your settings.json


or you can remove
"|pause"
from "support.function.ahk"
so Pause will never be a built-in function

@mark-wiemer
Copy link
Collaborator

As @FuPeiJiang mentioned, all function calls should be function color, as they are not being used as keywords when they are function calls. The function color depends on your VS Code theme: in @nmpsyntax's screenshots, function color is white. Key names (AKA keywords) should not take precedence over function calls, and I will update the extension to make all function calls function color.


I'm curious, is there any way to make
"name": "keyword.keys.ahk",
take precedence over
"name": "support.function.ahk",
?

I will look into that.


next time, please paste the code in the images, so someone else won't have to retype it

Re-iterating this point. Here is the code in the original screenshots, both screenshots contained in one code snippet:

<#Tab::     AltTab
<#c::       CenterActiveWindow()
$LAlt::     LAlt()
$*Pause::   Pause()
$AppsKey::  AppsKey()
$CapsLock:: CapsLock()

@nmpsyntax
Copy link
Author

Sorry about the code snippet, I've edited the original issue to include the code. I was trying to color the text like yours @mark-wiemer but wasn't sure how.

@mark-wiemer
Copy link
Collaborator

mark-wiemer commented Dec 12, 2020

No worries. You can color code in markdown by adding the language name to the end of the "fence" (the set of three backticks)

```autohotkey
Tab:: DoTabThing()
```

becomes

Tab:: DoTabThing()

@mark-wiemer
Copy link
Collaborator

@FuPeiJiang can you help me understand your reasoning for the changes to the regexes? For keywords.keys.ahk, is the best option really to double the length of a regex from ~1300 to ~2600 chars? And for entity.name.function.ahk you removed the protection that colored if and while as keywords even if the conditional expression is wrapped in parentheses:

while (x > 0) ; "while" should be keyword color, not function color
{
    x--
}

@FuPeiJiang
Copy link

reasoning:
if a keyword is followed by a ( (even with spaces \s in between), it is not a keyword anymore


I only added (?!\\s*\\() to the end of the regex, it is still ~1300 chars
do you mean that adding (?!\\s*\\() makes the regex 2x slower ?


if you are talking about "match": "(?i:^(\\s*\\w+)(?<!if|while)(\\()(.*)(\\))\\s*({)\\s*(;?.*)$)", (line 261), that's
"name": "functionline.ahk",

I was talking about changing
"match": "\\b(?!MsgBox)([\\w]+)(?=\\()", (line 282)
to
"match": "\\b(?!MsgBox)([\\w]+)(?=\\s*\\()",
which is also
"name": "entity.name.function.ahk",

@FuPeiJiang
Copy link

is there a way to match (syntax highlighting) without using regex ?

@mark-wiemer
Copy link
Collaborator

reasoning:
if a keyword is followed by a ( (even with spaces \s in between), it is not a keyword anymore

I only added (?!\\s*\\() to the end of the regex, it is still ~1300 chars
do you mean that adding (?!\\s*\\() makes the regex 2x slower ?

My mistake, I had copied your code wrong. Works fine, makes sense, merged to dev.

if you are talking about "match": "(?i:^(\\s*\\w+)(?<!if|while)(\\()(.*)(\\))\\s*({)\\s*(;?.*)$)", (line 261), that's
"name": "functionline.ahk",

I was talking about changing
"match": "\\b(?!MsgBox)([\\w]+)(?=\\()", (line 282)
to
"match": "\\b(?!MsgBox)([\\w]+)(?=\\s*\\()",
which is also
"name": "entity.name.function.ahk",

Gotcha, my mistake again. How is this change related to the original issue?

is there a way to match (syntax highlighting) without using regex ?

I'm not sure what you mean, could you provide some more details?

@FuPeiJiang
Copy link

how it's related:
regex: if keyword is a function, keyword is not keyword
but here, I've defined function as "LAlt     (", not only "LAlt("

here, "LAlt     (" is not a keyword, but it is not a function either
because function cannot have a space in between, so LAlt is : nothing, it's lightblue, a variable...
I'm matching functions with a space in between as functions (see the added \\s*)


are there other ways to do syntax highlighting than using ahk.tmLanguage.json ?
does tmLanguage provide if/else statements
because maybe i'd need if/else for complex matching

in ahk.tmLanguage.json,
"match" is always a regex,
"string.includes() matching with if/else" is faster than regex (for the computer)
not every matching needs to use regex

@mark-wiemer
Copy link
Collaborator

Gotcha, I will change line 282 as you mentioned.

RE syntax highlighting without regex: yep, the other core way is to compile the code and analyze the abstract syntax tree (AST). I have experience with compilers, but I have limited time to dedicate to this project. I know regex matching isn't great, but it gets the job done OK for now. You're more than welcome to search how to create a VS Code Language Support extension and see what you find, I'm sure there's plenty of documentation. But switching to AST analysis is no easy feat :). Let's not discuss alternative syntax highlighting methods on this issue anymore, it's a big enough discussion for its own thread.

@nmpsyntax
Copy link
Author

I must thank both of you for investigating this issue, but after reading the comment thread, I'm none the wiser as the how you intend to conclude this. Is Pause() going to be recognized as a keyword rather than a function call?

@mark-wiemer
Copy link
Collaborator

mark-wiemer commented Dec 14, 2020

@nmpsyntax No worries. All function calls will be "function color" regardless of their name. In your screenshots, "function color" looks white, the same color as CenterActiveWindow().

When you write LAlt(), you're not referencing the keyword LAlt, you're referencing a function you wrote named LAlt. Since it had the same name as a keyword, AHK++ thought you were referencing a keyword. But that was a bug, and a fix is merged into dev. Once I polish things up (likely late December), the fix will be released in a new version of AHK++.

Pause(), along with LAlt(), AppsKey(), Capslock(), and any other function call, will be colored white and recognized as a function call.

@mark-wiemer mark-wiemer added this to the December 2020 milestone Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working good first issue good for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants