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

#182 Restore cmdState after shell expansions. #183

Closed
wants to merge 3 commits into from

Conversation

zufuliu
Copy link
Contributor

@zufuliu zufuliu commented Jul 14, 2023

No description provided.

@nyamatongwe nyamatongwe added the bash Caused by the bash lexer label Jul 14, 2023
@zufuliu
Copy link
Contributor Author

zufuliu commented Jul 14, 2023

Fixed the test case (missing space before ]).
It maybe better to add test case for arithmetic expansion optimization: current == CmdState::Body && sc.Match('(', '(').

Don't highlight dash in long option as operator.
@zufuliu
Copy link
Contributor Author

zufuliu commented Jul 15, 2023

Test case for arithmetic expansion inside test command is added. dash in short/long command option no longer highlighted as operator.

@zufuliu
Copy link
Contributor Author

zufuliu commented Jul 15, 2023

Also I think it's better to move IsAlpha(), IsIdentifierChar() and IsIdentifierStart() from LexOScript.cxx to CharacterSet.h, many IsASCII(ch) && isalpha(ch) could be replaced with IsAlpha(ch).

@nyamatongwe
Copy link
Member

IsAlpha is available from CharacterSet.h with the name IsUpperOrLowerCase. IsAlpha is defined differently in LexMagik.cxx.

Add test case for long option highlighting.
@zufuliu
Copy link
Contributor Author

zufuliu commented Jul 15, 2023

OK, replaced IsAlpha() with IsUpperOrLowerCase().

@nyamatongwe nyamatongwe added the committed Issue fixed in repository but not in release label Jul 15, 2023
@nyamatongwe
Copy link
Member

Languages often allow a few punctuation characters like - and % in identifiers so I'm uncomfortable with pushing C's original A-Za-z_ rule. Even for C, $ is a popular extension which is supported by the cpp lexer.

So, I'd prefer a name that describes what it does (IsAlphaOrUnder?) for the header rather than describing its role. Lexers can add their own IsIdentifierChar or similar that can then be modified locally as needed.

@zufuliu zufuliu deleted the LexBash-Issue182 branch September 24, 2023 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bash Caused by the bash lexer committed Issue fixed in repository but not in release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants