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

Fix keyword colorization on filetype change #3553

Merged
merged 1 commit into from Oct 6, 2023

Conversation

techee
Copy link
Member

@techee techee commented Sep 10, 2023

When filetype is changed to a compatible filetype (such as C->C++ and then possibly back to C), colorization is lost. It seems that with Scintilla 5, when language-specific highlighting changes, SCI_SETKEYWORDS has to be called again to take effect.

By setting keyword_hash to 0 we force Geany to call sci_set_keywords() in document_highlight_tags() which fixes the problem.

Fixes #3550

When filetype is changed to a compatible filetype (such as C->C++ and
then possibly back to C), colorization is lost. It seems that with
Scintilla 5, when language-specific highlighting changes, SCI_SETKEYWORDS
has to be called again to take effect.

By setting keyword_hash to 0 we force Geany to call sci_set_keywords() in
document_highlight_tags() which fixes the problem.
@kugel-
Copy link
Member

kugel- commented Sep 11, 2023

Have you checked if this is perhaps a bug in scintilla and maybe fixed by #3551?

@elextr
Copy link
Member

elextr commented Sep 11, 2023

See also analysis here

@kugel-
Copy link
Member

kugel- commented Sep 11, 2023

I read that but it's not clear to me how that explains the change here. You talk about a failed parser but that is not talked about here. So I assumed @techee came to a different conclusion.

@elextr
Copy link
Member

elextr commented Sep 11, 2023

@kugel- When you posted a suggestion about possible cause/fix here on the pull request, not the issue #3550 where the discussion had happened, it was not clear that you had seen that discussion, so I posted a cross reference to be helpful.

The cross referenced post actually agrees with @techee as to the cause and adds detail of the underlying triggers for the bug based on my analysis, so yes it says things not mentioned by @techee. But it is not disagreeing with his identification of the bug.

@techee
Copy link
Member Author

techee commented Sep 11, 2023

Have you checked if this is perhaps a bug in scintilla and maybe fixed by #3551?

I just tried and no, it doesn't fix it unfortunately.

It's hard to say whether this is really a bug on the Scintilla side - I didn't investigate what exact Scintilla API gets called in Geany when filetypes change (one would have to go through the many calls in highlighting.c and see which one triggers it but it's not worth it). At the end it might just be some undefined behavior which worked in previous Scintilla versions but which doesn't work now.

@zufuliu
Copy link

zufuliu commented Sep 28, 2023

It's hard to say whether this is really a bug on the Scintilla side

I guess this due to SCI_SETILEXER: each time it will set a new lexer for current document even if it has same identifier (SCI_GETLEXER) as previous one.

@techee techee added this to the 1.39/2.0 milestone Oct 5, 2023
@techee
Copy link
Member Author

techee commented Oct 5, 2023

I've just added the 2.0 milestone here because this affects the ProjectOrganizer plugin and type colorization there.

@elextr
Copy link
Member

elextr commented Oct 6, 2023

LGBI

The diagnosis of @zufuliu above makes sense, a new lexer will always need new keywords to be set.

This may have been a bug for always, but not exposed until PO started swapping filetypes with abandon.

@techee
Copy link
Member Author

techee commented Oct 6, 2023

OK, merging.

@techee techee merged commit c1c4715 into geany:master Oct 6, 2023
3 of 4 checks passed
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.

Typenames not colorized after updating to Scintilla 5
4 participants