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

Update lexilla to 5.2.4 #3441

Closed
wants to merge 1 commit into from
Closed

Update lexilla to 5.2.4 #3441

wants to merge 1 commit into from

Conversation

Azq2
Copy link

@Azq2 Azq2 commented Apr 1, 2023

No description provided.

@elextr
Copy link
Member

elextr commented Apr 2, 2023

I can't test ATM, but looks good by inspection, all new styles seem to have made it into highlightingmappings.h, well done.

I had a thought while inspecting that filetypes files should include all lexer property settings, even if set to Lexilla default or commented out, since Lexilla only code-documents them. I will open an issue for it. Maybe you could add them to the four filetypes you modify here (but not any others).

@rdipardo
Copy link
Contributor

rdipardo commented Apr 2, 2023

@elextr,

I had a thought while inspecting that filetypes files should include all lexer property settings, even if set to Lexilla default or commented out, since Lexilla only code-documents them.

The SciTE website lists every known property, including lexer properties. To your point, though, it's not very searchable.

Lexilla maintains a Python script to keep the properties table in sync with code changes:

https://github.com/ScintillaOrg/lexilla/blob/43ea736569d52ba6cf7e7325cf39009409e7282b/scripts/LexillaData.py#L19

#2517 was an attempt to automate the extraction of property info from release tarballs.

@eht16
Copy link
Member

eht16 commented Apr 16, 2023

I wonder if it is OK to just update Lexilla and not Scintilla as well?

But probably better than nothing and we can update Scintilla afterwards anyway.

@elextr
Copy link
Member

elextr commented Apr 16, 2023

I guess updating one without the other is why Neil split them. So long as no lexer needs a capability only available in a newer Scintilla it should be ok.

@kugel-
Copy link
Member

kugel- commented Apr 17, 2023

I hope to have a look here after my vacations in about two weeks

@elextr
Copy link
Member

elextr commented May 28, 2023

ping @kugel-

@kugel-
Copy link
Member

kugel- commented May 29, 2023

Really sorry, but I'm in the middle of moving to a new home. I won't have spare time for another week or three.

@rdipardo
Copy link
Contributor

GDScript's new annotation style has not been fully integrated yet.

The file type definition still contains a decorator=decorator mapping. As a result, annotations don't show in GDScript files (lines 49, 51):

gdscript-lexilla-524-no-annotations

This appears to be a trivial fix:

diff --git a/data/filedefs/filetypes.gdscript b/data/filedefs/filetypes.gdscript
index 9b3b63951..d2ca49ffe 100644
--- a/data/filedefs/filetypes.gdscript
+++ b/data/filedefs/filetypes.gdscript
@@ -19 +19 @@ word2=keyword_2
-decorator=decorator
+annotation=decorator

gdscript-lexilla-524-with-annotations

@kugel-
Copy link
Member

kugel- commented Jul 18, 2023

@rdipardo good catch. However, this seems to be an artifact of an earlier lexilla update. Would you mind creating a separate PR for that?

@Azq2 Why did you only update lexilla? We typically update both components together. And regardless, there is lexilla 5.2.5. Can you please target that?

@rdipardo
Copy link
Contributor

rdipardo commented Aug 6, 2023

@kugel-

this seems to be an artifact of an earlier lexilla update.

I checked and in fact the GDScript filedef was never quite correct. It was copied almost verbatim from the Python filedef without a significant change since being added in f59e520.

A point-in-time build based on f59e520 is missing the annotation style the same as I noted earlier:

geany-git-f59e520-gdscript

Would you mind creating a separate PR for that?

@Azq2's fork is a good enough place to finally correct the file def, I think. I'm loath to clutter the PR queue with a one-line diff. In any case it would have to follow a merged update to Lexilla and/or Scintilla, and @Davidy22 may end up getting there first: #3111 (comment)

@elextr
Copy link
Member

elextr commented Sep 16, 2023

Works for me on top of #3551 but with very limited testing (C/C++/Python). @kugel- have you had a chance to review?

@kugel-
Copy link
Member

kugel- commented Sep 16, 2023

#3551 already includes lexilla 5.2.6, no need to apply this on-top.

This the author isn't replying anyway I would just close this. We must not forget the GDScript fix though.

@techee
Copy link
Member

techee commented Oct 23, 2023

We already have a newer version of Scintilla. Closing.

@techee techee closed this Oct 23, 2023
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.

None yet

6 participants