Skip to content

TOML, Dart, Zig, Nix: Add error state for unterminated string#315

Closed
techee wants to merge 4 commits intoScintillaOrg:masterfrom
techee:error_state
Closed

TOML, Dart, Zig, Nix: Add error state for unterminated string#315
techee wants to merge 4 commits intoScintillaOrg:masterfrom
techee:error_state

Conversation

@techee
Copy link
Copy Markdown
Contributor

@techee techee commented May 11, 2025

For single-line strings it's easy to forget the termination character for these languages. This PR tries to address that by adding the SCE_..._STRINGEOL state marking unterminated strings similarly to other languages.

I tried to follow the implementation of other lexers - typically the implementation looks like

if (sc.state == STRING && sc.atLineEnd) {
    sc.ChangeState(STRINGEOL);
    sc.ForwardSetState(DEFAULT);
}

However, the sc.ForwardSetState() at line end isn't quite correct - many lexers contain

if (sc.atLineEnd) {
    styler.SetLineState(sc.currentLine, lineState);
    ...
}

at the end of the main loop where they set line state and update various state variables and sc.ForwardSetState() when handling unterminated string may cause this code to be skipped.

I only ran into this problem with the TOML lexer and the provided unit test so I implemented "correctly" only this lexer; I left the "incorrect" implementation (which is used by many existing lexers too) with the remaining lexers. It probably doesn't matter much as unterminated strings are supposed to be fixed by users soon and not to remain in the code for a long time.

@nyamatongwe nyamatongwe added toml Caused by the TOML lexer dart Caused by the Dart lexer zig Caused by the Zig lexer nix Caused by the Nix lexer labels May 12, 2025
@nyamatongwe
Copy link
Copy Markdown
Member

It should be fairly easy to avoid the ForwardSetState and thus the potential for new interesting bugs.

Something like switching state near the top of the loop from SCE_..._STRINGEOL to SCE_..._DEFAULT if sc.atLineStart.

@techee
Copy link
Copy Markdown
Contributor Author

techee commented May 13, 2025

Something like switching state near the top of the loop from SCE_...STRINGEOL to SCE..._DEFAULT if sc.atLineStart.

You are right, it works this way. I actually tried that before and then got some strange error messages from the unit test runner regarding \r and \n but I tried it again now and it works correctly. Apparently I did something wrong before.

I've repushed the commits with the updated version.

@nyamatongwe
Copy link
Copy Markdown
Member

Committed with mentions in history and pull-number tags.

@nyamatongwe nyamatongwe added the committed Issue fixed in repository but not in release label May 13, 2025
@techee
Copy link
Copy Markdown
Contributor Author

techee commented May 14, 2025

Great, thanks!

@nyamatongwe nyamatongwe removed the committed Issue fixed in repository but not in release label Jun 8, 2025
@nyamatongwe nyamatongwe closed this Jun 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dart Caused by the Dart lexer nix Caused by the Nix lexer toml Caused by the TOML lexer zig Caused by the Zig lexer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants