Skip to content

Fix lexing of nested comments; remove unused variable#258

Closed
mgusg wants to merge 6 commits intoScintillaOrg:masterfrom
mgusg:master
Closed

Fix lexing of nested comments; remove unused variable#258
mgusg wants to merge 6 commits intoScintillaOrg:masterfrom
mgusg:master

Conversation

@mgusg
Copy link
Copy Markdown

@mgusg mgusg commented Jul 25, 2024

Fix a bug which caused improper lexing of nested block comments.
Add a regression test for block comments and single-line comments.
Remove unused variable visibleChars in LexerABL::Lex.

@nyamatongwe nyamatongwe added the progress Caused by the abl (progress) lexer label Jul 25, 2024
@nyamatongwe
Copy link
Copy Markdown
Member

The test requires a SciTE.properties file in the same directory to specify the lexer to test and turn on folding. Something similar to

lexer.*.p=abl
fold=1

Removing visibleChars means that the local function IsSpaceEquiv is no longer called and should be removed to avoid warnings.

../lexers/LexProgress.cxx:51:9: warning: 'bool {anonymous}::IsSpaceEquiv(int)' defined but not used [-Wunused-function]
   51 |    bool IsSpaceEquiv(int state) {
      |         ^~~~~~~~~~~~

cppcheck shows a new warning for the look back loop. It may be possible to simplify this code.

lexilla\lexers\LexProgress.cxx:271:14: warning: Condition 'checkIsSentenceStart' is always true [knownConditionTrueFalse]
         if (checkIsSentenceStart && st != SCE_ABL_COMMENT && st != SCE_ABL_LINECOMMENT && st != SCE_ABL_CHARACTER  && st != SCE_ABL_STRING ) {
             ^
lexilla\lexers\LexProgress.cxx:264:27: note: Assuming that condition 'checkIsSentenceStart' is not redundant
      while (back >= 0 && checkIsSentenceStart) {
                          ^
lexilla\lexers\LexProgress.cxx:271:14: note: Condition 'checkIsSentenceStart' is always true
         if (checkIsSentenceStart && st != SCE_ABL_COMMENT && st != SCE_ABL_LINECOMMENT && st != SCE_ABL_CHARACTER  && st != SCE_ABL_STRING ) {
             ^

@mgusg
Copy link
Copy Markdown
Author

mgusg commented Jul 26, 2024

Thanks for the feedback. I added the missing SciTE.properties file and resolved the issues reported by cppcheck.

@nyamatongwe
Copy link
Copy Markdown
Member

In SciTE.properties you likely want the keyword 'DISPLAY' to be lowercase so that these are lexed to SCE_ABL_WORD (2) instead of SCE_ABL_IDENTIFIER (7). This lexer implements case-insensitive keywords by converting words to lowercase then searching in a list. This will never match 'DISPLAY'.

@mgusg
Copy link
Copy Markdown
Author

mgusg commented Jul 29, 2024

I changed the keyword to lowercase and updated the .styled file. Thank you for the thorough review.

nyamatongwe pushed a commit that referenced this pull request Jul 29, 2024
@nyamatongwe nyamatongwe added the committed Issue fixed in repository but not in release label Jul 30, 2024
@nyamatongwe nyamatongwe removed the committed Issue fixed in repository but not in release label Feb 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

progress Caused by the abl (progress) lexer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants