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

Inno: Multiline comments and close with correct comment sequence #44

Closed
mpheath opened this issue Dec 3, 2021 · 4 comments
Closed

Inno: Multiline comments and close with correct comment sequence #44

mpheath opened this issue Dec 3, 2021 · 4 comments
Labels
inno Caused by the inno setup lexer

Comments

@mpheath
Copy link
Contributor

mpheath commented Dec 3, 2021

The Inno lexer uses default styling on multiline comments.

The C-like comment of

// ...

are single line comments, like in C.

The Pascal comments behave like the C type of /* ... */ comment

// Single line comments:

// ...

{ ... }

(* ... *)

// Multiline comments:

{ ...
... }

(* ...
... *)

which can span multiple lines.

The current Inno lexer finds the EOL on the first comment line and so it may consider { ... or (* ...
as default style if not closing } or *) is found. The second comment line is going to be default style unless it starts with a known sequence to set a non default style.

In explaining this, I just thought of a bug as the inno lexer allows these comments

(* ... }

{ ... *)

as the code does not remember the opening sequence to know which closing sequence is required. Added bool isCommentCurly, bool isCommentRound and renamed bool isCStyleComment to bool isCommentSlash. Now the closing sequence required can be known.

Needed to use the line state functions to handle the multiline comments. Removed the enum section{...} as the related error messages were long and confusing so changed to more basic int const bitCode, bitMessages, bitCommentCurly, bitCommentRound names.

Minor tidy done with parameter spacing in the source which I hope is OK.

The behaviour is better so that comments and strings are styled during typing. Eliminated some of the go back a char with the styler.

Updated the test file x.iss with the x.iss.folded and x.iss.styled files. Snapshot included as x.png which can be discarded after viewing.

This is just the area with the comments:

c.png

Attached files have CRLF line ends:

inno.zip

Log:

  • Fixed Inno comment ends with incorrect closing sequence.
  • Added Inno styling of multiline comments.
@nyamatongwe nyamatongwe added the inno Caused by the inno setup lexer label Dec 4, 2021
@nyamatongwe
Copy link
Member

This appeared OK and passed tests on Windows so it was committed. However, it produces inconsistent results on Unix as shown in the Linux action https://github.com/ScintillaOrg/lexilla/runs/4479455090?check_suite_focus=true

If run on a Unix system, a problem occurs with styling on line 39, the first multiline comment with the comment not flowing onto the next line.

{7}(*
{0}comment *)

It occurs when styling the file line by line with Unix-style '\n' line ends, not Windows '\r\n' line ends.

I have not been able to reproduce this interactively yet.

@mpheath
Copy link
Contributor Author

mpheath commented Dec 10, 2021

TestLexer.exe is quite useful for detecting issues. I did not consider a test file for each EOL sequence, as the styling was good in the editor and the CRLF test file, yet it has issues that TestLexer.exe can detect with the test file with the \n EOL sequence.

  • With (*\r\n, push forward to the asterisk is causing the \r to not be processed.
  • With (*\n, push forward to the asterisk is causing the \n to not be processed.

Push forward is causing the character after the * to not be processed. Even the (* comment *)\r\n is causing the space after the * to not be processed.

If \n is not processed when \n is the EOL sequence, then that appears to affect the line state setting as it changes state on each \n character.

I have tried different variations to push forward, though same results happen. Removal of the push forward seems the only reasonable solution. The push forward is not needed for the this lexer as processing the following asterisk or slash should not cause an issue.

Difference:

--- a/LexInno.cxx
+++ b/LexInno.cxx
@@ -151,18 +151,12 @@
 					// Start of a Pascal comment
 					state = SCE_INNO_COMMENT_PASCAL;
 					isCommentRound = true;
-					// Push forward to the asterisk
-					ch = chNext;
-					chNext = styler[++i];
-					styler.ColourTo(i, SCE_INNO_COMMENT_PASCAL);
+					styler.ColourTo(i + 1, SCE_INNO_COMMENT_PASCAL);
 				} else if (isCode && ch == '/' && chNext == '/') {
 					// Start of C-style comment
 					state = SCE_INNO_COMMENT_PASCAL;
 					isCommentSlash = true;
-					// Push forward to the next slash
-					ch = chNext;
-					chNext = styler[++i];
-					styler.ColourTo(i, SCE_INNO_COMMENT_PASCAL);
+					styler.ColourTo(i + 1, SCE_INNO_COMMENT_PASCAL);
 				} else if (!isMessages && ch == '"') {
 					// Start of a double-quote string
 					state = SCE_INNO_STRING_DOUBLE;

Attached files have CRLF line ends:

inno2.zip

@nyamatongwe
Copy link
Member

Committed the above as a27851f.

Also added testing of line-by-line lexing with Unix line ends so problems will be discovered on Windows before committing in the future. This isn't symmetrical - Windows line ends aren't checked on Unix yet.

@nyamatongwe
Copy link
Member

Included in 5.1.5 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inno Caused by the inno setup lexer
Projects
None yet
Development

No branches or pull requests

2 participants