10000 Ace Javascript Highlight Mode: Highlight issue · Issue #5257 · ajaxorg/ace · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Ace Javascript Highlight Mode: Highlight issue #5257

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

Jump to bottom
Closed
CodeSmith32 opened this issue Jul 17, 2023 · 2 comments
Closed

Ace Javascript Highlight Mode: Highlight issue #5257

CodeSmith32 opened this issue Jul 17, 2023 · 2 comments
Labels
bug effort/small mode-issue p2

Comments

@CodeSmith32
Copy link

Describe the bug

It appears Ace's JS highlight mode has trouble correctly highlighting the arrow function arguments in the following pieces of code:

// works as expected
func(a,b,1,2,3)
func(a,b,innerCall(c,d))
(a=1,b=2) => {}
function* generator() {}

// does not work
func(a,b,1,2,3, () => {})
func(a,b,innerCall(c,d), () => {})
(a=(1),b=(2)) => {}
function *generator() {}
function*generator() {}

Expected Behavior

I would expect a, b, and innerCall to be marked with the class ace_identifier since they are variable names, not argument names.

I would also expect b in the arrow function to appear as an argument name (css classes ace_variable ace_parameter), and not an identifier.

I would also expect the generator function to highlight as a function in all cases.

Current Behavior

Instead, the highlighting is not correct:

issue-sample

Reproduction Steps

Using the Ace editor, simply paste the code:

// works as expected
func(a,b,1,2,3)
func(a,b,innerCall(c,d))
(a=1,b=2) => {}
function* generator() {}

// does not work
func(a,b,1,2,3, () => {})
func(a,b,innerCall(c,d), () => {})
(a=(1),b=(2)) => {}
function *generator() {}
function*generator() {}

Possible Solution

This is, of course, just an issue with the javascript_highlight_rules. It's located here:
https://github.com/ajaxorg/ace/blob/master/src/mode/javascript_highlight_rules.js#L420

It appears the rule here is too greedy, grabbing anything after a ( character and processing it as function arguments. This, I understand, is because arrow function arguments can have default values which contain parentheses. To fix it would potentially require finding the beginning parenthesis matching the one before the arrow =>, although I can't say what that looks like.

The reason b in the arrow function is not highlighted as an argument, though, is actually because the default_parameter mode, when it fails to match, reverts back to no_regex (the default mode) which never returns to function_arguments:
https://github.com/ajaxorg/ace/blob/master/src/mode/javascript_highlight_rules.js#L324

This is affecting all function-types, not just arrow functions, e.g., function func(a=(1),b=(2)) {}. Does Ace's highlight parser support a mode stack yet? e.g., a mode can be entered, but when leaving it, it falls back to the 'previous' mode instead of always going to a specific one. I don't see a way around this issue until a mode stack is supported. (I also always found this lack rather frustrating when implementing my own syntaxes)

Lastly, please allow whitespace between function and * for generator functions, and no whitespace between the * and the function name, both of which are valid in javascript:
https://github.com/ajaxorg/ace/blob/master/src/mode/javascript_highlight_rules.js#L124 (there are many other occurrences)

This isn't as hard as the above issues, and could easily be solved with a regex piece like,

"(function\\s*(?:\\*|(?=\\s)))(\\s*)" // always using \\s* at the end, vs. the \\s+ in some places

Additional Information/Context

No response

Ace Version / Browser / OS / Keyboard layout

Ace v1.23.4 / All Browsers / All OSes / Not applicable
@whazor
Copy link
Contributor
whazor commented Jul 27, 2023

We would certainly welcome a pull request to resolve this issue. Maybe it would need some tests as well

@mkslanc mkslanc mentioned this issue Apr 23, 2024
Merged
1 task
@akoreman
Copy link
Contributor

Fixed as part of #5538, will be included in the next Ace release after 1.33.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug effort/small mode-issue p2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@whazor @CodeSmith32 @akoreman
0