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

Speedup lexers #118

Closed
wants to merge 3 commits into from
Closed

Speedup lexers #118

wants to merge 3 commits into from

Conversation

mcepl
Copy link

@mcepl mcepl commented Oct 3, 2024

This is just rebasing of #15, #16, and #17 on the top of the current master.

I have been running this for the past couple of weeks on the top of vis, and it doesn't seem to break anything. I haven’t measured the speed difference, but even with large XML files (Bible text, approx. 15MB) the editor was very swift.

This adds performance improvements added to vis in commits:
7e9e0a2ca868aaa214fb38a79fe71da34d6e00da and
baa51e934ce057af5b5be829d6a73a3e8b4c03d0
This fix is committed in baa51e934ce057af5b5be829d6a73a3e8b4c03d0
and is the same as used in the xml lexer.
Adds commit baa51e934ce057af5b5be829d6a73a3e8b4c03d0 which is the
same performance fix as used by the wsf and xml lexer.
@orbitalquark
Copy link
Owner

Thanks, I'll take a look when I have some time.

@orbitalquark
Copy link
Owner

Thanks again for submitting this. To be clear, the lexers were already fast, as there was no expensive tag test to highlight '=' and number attribute values -- it could be a bit sloppy.

This PR adds an inexpensive postfix (look-ahead) tag test. The commented out test was a prefix test with look-behind.

After experimentation, there are a couple issues I have with this change.

  1. Normally, Scintillua matches entities with the closing delimiter as optional (strings, block comments, etc.). This change goes against that. Something like <div class="foo" will not highlight properly until the closing > is added. You won't see this if your editor automatically adds a closing '>' when typing '<'.
  2. Some editors start lexing at the start of a line, so building on the point above, a multi-line tag will never highlight attributes properly, even after typing a closing '>'. For example:
<div
class="foo"
style="float: left;"

Typing a '>' on the 'style' line will highlight only that line and leave the 'class' line alone, and typing a '>' after the 'style' line will not highlight either attribute line.

The first issue I could perhaps be okay with, as technically a tag is not valid until it is closed, but the second issue is more problematic, because we cannot assume an editor inserts a trailing '>' automatically, so Scintillua may not highlight a finished tag.

@mcepl
Copy link
Author

mcepl commented Oct 15, 2024

I have no problem if you decide that you don’t want these changes (I am quite certain, that you understand intricacies of Scintillua lexers a way more than me and @moesasji combined), but then it would be probably better to make the decision and close all those PRs.

@orbitalquark
Copy link
Owner

Okay, I will close the PRs then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants