LexInno: Fix style handling of unterminated strings, message sections, and avoid terminating comments between CR and LF

[mpheath]

Fixed issues with Parameters: and with Keywords= with anchoring the words to trailing : and = characters.

Tested with https://github.com/XhmikosR/notepad2-mod/blob/master/distrib/notepad2_setup.iss

Note the emphasised bolding in fixes.

Fixed in Setup section:

WizardSmallImageFile=WizardSmallImageFile.bmp

to

WizardSmallImageFile=WizardSmallImageFile.bmp

Value should be default style in this case.

Fixed in CustomMessages section:

en.msg_DeleteSettings =Do you also want to delete {#app_name}'s settings?%n%nIf you plan on installing {#app_name} again then you do not have to delete them.

and

en.tsk_Other =Other tasks:

and

en.tsk_ResetSettings =Reset {#app_name}'s settings

Unterminated strings should be Default style in CustomMessages and Messages sections. tasks should be Default style, not Parameter style.

Fixed in Tasks and InstallDelete sections:

Name: quicklaunchicon; Description: {cm:CreateQuickLaunchIcon}; GroupDescription: {cm:AdditionalIcons}; Flags: unchecked; OnlyBelowVersion: 6.01

and

Type: files; Name: {#quick_launch}{#app_name}.lnk; Check: not IsTaskSelected('quicklaunchicon') and IsUpgrade(); OnlyBelowVersion: 6.01

OnlyBelowVersion should be Parameter style, not as Keyword style. Even though in latest inno.properties, may be coloured the same.

Note

The test inno script has no Registry section to show the fix of String: as Parameter style in INI section and string; as Default style in Registry section, which is what lead to to these fixes as mentioned at SourceForge.

[mpheath]

Tried to create name isMessages to boolean for Messages and CustomMessages sections similar to existing isCode name for Code section. isMessages does change from false to true like isCode does, though no matter where I put the isMessages condition, it has no effect on the styling. The buffer may need adjustments and anything I tried still had no effect.

Before the for loop, I declared isMessages:

bool isMessages = false;

and then where isCode is tested :

isCode = !CompareCaseInsensitive(buffer, "code");

if (isCode) {
    isMessages = false;
} else {
    isMessages = !CompareCaseInsensitive(buffer, "custommessages");

    if (!isMessages)
        isMessages = !CompareCaseInsensitive(buffer, "messages");
}

That sets isMessages to true or false. Tested with printf() output and changes to true and false correct.

Something is going on in the for loop that I do not understand. I suspect it has something to do with the buffer. My experience with C++ is little. Perhaps someone with better experience than me knows how to do this.

[nyamatongwe]

Lexers are called incrementally over portions of the file as text is scrolled into view. State set up for one portion does not automatically flow to the next portion. Any state that must be remembered for a line can be placed in line state. See how SetLineState and GetLineState save and restore isCode. Use a second bit to store isMessages.

[nyamatongwe]

Changes to lexers should include a simple testable example file under test/examples/{lexer}. This allows automatic testing of lexers to prevent regressions and also makes it easier for future maintainers to work out just what was changed, The automatic tester is documented in test/README

I have added a small example test/examples/inno/x.iss. Unfortunately, LexInno currently has problems with the automated tester which wants identical results when line ends are changed from \n (Unix) to \r\n (Windows) or vice-versa. LexInno treats comments differently for Unix and Windows line ends, placing a style change between the \r and \n. Because of this, the example does not contain comments.

There was an attribute problem with x.iss initially so it was not marked good until 3e64a18.

[zufuliu]

I happened to wrote a lexer for Inno Setup 6.2 recently, source (not compiles with lexilla's lexlib) is at https://github.com/zufuliu/notepad2/blob/master/scintilla/lexers/LexInno.cxx, you can test latest builds for notepad2 from https://github.com/zufuliu/notepad2/actions or https://ci.appveyor.com/project/zufuliu/notepad2.

Following missing features are implemented:

1. highlighting and folding for preprocessor (multiline function defines, /*c block comment*/, <include path>, etc.).
2. folding for pascal code in the Code section.
3. highlighting for pascal code after Check and Components parameters.
4. folding for consecutive comment lines.
5. nested inline expansion.
6. highlighting for %1, %2, %n like replacements/parameters.

The code is written from scratch (using StyleContext, not ported into DefaultLexer), if the big change is accepted, I can take time to backport it to lexilla.

[mpheath]

Multiple fixes achieved for the inno lexer.

• Keyword and parameter anchored to : and =. (Already pushed)
• Restrict styling in CustomMessages and Messages section. (Thanks for the advice)
• Inno comments, strings and pascal c-type comment prevent stying EOL. (As you reported)
• Single quoted strings allow backslash escapes. (:thumbsup:)

The EOL as default style fixed for both \n and \r\n. InnoSetup is Windows based installer though I tested a compile with \n EOL and it compiled and ran as normal.

Would you like me to commit these separately into this PR? If you are going to squash commit then it may not matter, though may make viewing them easier if separate.

This is before and after fixes:

Bozo colours as the person who did the Lua styling commented. Helps to highlight the issues.

Script:

#define app_copyright "Copyright 1999, app corporation"

; comment
; comment

[Setup]
AppName=MyApp
AppCopyright={#app_copyright}
WizardSmallImageFile=WizardSmallImageFile.bmp
OnlyBelowVersion=6.01

[Files]
Source: "app.exe"; DestDir: "{tmp}"; OnlyBelowVersion: 6.01

[INI]
Key: "version"; String: "1.0"; "unterminated
Key: "version"; String: "1.0"; 'unterminated

[Registry]
Root: HKLM; ValueType: string

[CustomMessages]
keyname                 =Other tasks:'not string

[Messages]
keyname="{#app_copyright}"not string

[Code]

// comment
// comment

(* comment *)
(* comment *)

function ShouldInstallComCtlUpdate: Boolean;
begin
  Result := False;
  Log('string');
  IsEscaped('\'good', '\\\'good', '\\'bad');
end;

[nyamatongwe]

Would you like me to commit these separately into this PR?

I generally preferred isolated separate commits for each feature or fix as that makes it easier to trace bugs back and understand what was changed and why.

[mpheath]

@nyamatongwe Pushed all fixes I have. I get 404 clicking on the committed links list here. Perhaps does not like the leading hash number. My branch history is working currently to see the separate diffs. In Files changed tab here, have a combobox to select to view each commit which is an alternate way to see each diff.

[nyamatongwe]

These changes have been committed with test cases added.

8971000 made the automatic tester work as properties had to be applied to CRLF tests.

[zufuliu]

I think C style backslash escape sequences should be disabled by default, as it's only enabled after #pragma parseroption -p- (or with /Pp- command line option), see https://jrsoftware.org/ispphelp/index.php?topic=pragma.

See examples at https://github.com/jrsoftware/issrc/blob/main/Examples/AllPagesExample.iss#L76

 InputDirWizardPage.Values[0] := 'C:\';

Edit: the option is -p-, p+ is enable pascal string.

Pascal-style string literals. In off state, uses C-style string literals (with escape sequences). Default state: on.

https://github.com/jrsoftware/issrc/blob/main/Files/ISPPBuiltins.iss#L22

[mpheath]

I think C style backslash escape sequences should be disabled by default, ...

Thanks for checking that pragma option. You are correct with your thinking.

As mentioned at FreePascal:

The single quote character can be embedded in the string by typing it twice. The C construct of escaping characters in the string (using a backslash) is not supported in Pascal.

C-style esacaping is only a inno pragma option which is not enabled by default.

StackOverflow also has a topic on it.

My error in thinking C-style escapes were default. :(

I may need to commit 2 more fixes.

• fix by undo the 'Single quoted strings allow backslash escapes' as doubled single quotes need no additional conditions for styling.
• fix unittest files

I need a little time to test to ensure this is done correct. I am not sure if the lexer can account for inno pragma options if not set with the default. I have no current knowledge of handling inno pragma options in the lexer code.

[mpheath]

@nyamatongwe I pulled the PR and x.iss was missing a final newline which I did another commit to resolve. Without, TestLexers.exe will create *.new files. The conflict I had to resolve may have contributed to losing the newline. Sorry for the extra effort needed by you to fix this.

[nyamatongwe]

Reverted the backslash change with e3d269b which should be equivalent to the 4 above commits.

[mpheath]

Looks good to me. TestLexers.exe passed. LexInno.cxx diffs good. The branch conflict probably does not need fix as the commits are done. If you agree is OK, can close the PR as done when you are ready.

Perhaps the closed PRs that have individual commits to master could be labeled as MergesDone, CommitsDone or something similar so as not to appear as a rejected PR. Just an idea.

Reminder: SciTEHistory.html has backslash escapes in strings, could be removed to prevent confusion as the feature was reverted.