Remove the external lexer support due to Scintilla 5

[rdipardo]

maybe plugin authors could start rolling their own lexilla.dll [. . .] ?

Nope: they went the static route with Lexilla, too.

You could try upvoting this upstream issue; but it won’t help until N++ can load properties at runtime the way SciTE does.

[mpheath]

@rdipardo

I am not sure if statically linked is the problem. The image I posted previously is the Sc1 version of SciTE (statically linked with Scintilla and Lexilla). If Sc1 can do it, so I would expect also for Notepad++.

The upstream issue you mention is about Scintillua with Sc1. Basically wants an LPeg lexer priority before Lexilla lexer. If Neil knows the patch is good, it would have been committed. I do not use Scintillua and the other poster has not returned a response to confirm if the patch is OK. Not sure if this issue is related to Notepad++ and it’s external lexing as Notepad++ does not use Scintillua.

[Ekopalypse]

Scintilla still supports external lexers, but it won’t work without support from Npp.
I have asked Npp’s position on this in the commit.

[rdipardo]

@mpheath

The image I posted previously is the Sc1 version of SciTE (statically linked with Scintilla and Lexilla).

After a bit of grokking it works for me, too (*). I’ve automated the steps for anyone who wants to try it.

You’ll need git, mercurial and CMake all in your PATH, and at least Visual Studio 2017 with the C++ workload installed.

Start the “x64 Native Tools” or “x86_x64 Cross Tools” Developer Command Prompt and run:

git clone --recursive -b scite-gtk3 https://bitbucket.org/rdipardo/lexilla-dev.git
cd lexilla-dev\demo
.\sc1-with-ext-lexer.bat

---

(*) I usually let MsBuild choose the default platform of x86, but a 32-bit DLL will export decorated functions that won’t match the definitions in Lexilla.h, and so fail to load.

[rdipardo]

@Bas-de-Reuver, @Ekopalypse,

Remove Replace the external lexer support due to Scintilla 5 with the new Lexilla interface

https://github.com/notepad-plus-plus/notepad-plus-plus/pull/11468

[Stan Mitchell130]

@bas-de-reuver
I just read your post. As an author of an external lexer plugin, I too am concerned about the impact of removing external lexer support. When I did my last release of GEDCOM lexer 3 years ago, I thought about submitting the lexer to Scintilla so it would become an “internal” lexer. Maybe that is a path forward if all else fails.

[mpheath]

@stan-mitchell130

In case you missed it… @rdipardo posted with a link showing external lexer restoration:

Restore external lexer library support #11468

and has been merged recently into the repository with the message:

Make external language library work again after upgrading to Scintilla5

Make external language library work again after upgrading to Scintilla5
Make external lexer library work again after upgrading to Scintilla5.
Old external lexer libraries needs to add CreateLexer export function which returns ILexer5 instance (Lexilla protocol interface of Scintilla5).
Tested with papyrus lexer plugin, this external lexer plugin is compatible with Notepad++ next release:
https://github.com/blu3mania/npp-papyrus

External lexers were removed temporarily. Now, have been added back in as ILexer5 compatible!

[Stan Mitchell130]

@mpheath
Thanks for the update! I’ll have to dig into the ILexer5 changes.

[Bas de Reuver]

I was looking at this example, and I added a method called CreateLexer to the file UnmanagedExports.cs file of the C# LexerExample.

With this added, the plugin works again, there is no error message (“Loading CreateLexer function failed. Edifactlexed.dll is not compatible with the current version of Notepad++.”) anymore at startup of Notepad++, and the syntax highlightng also still works. 😃

@Ekopalypse Do I understand correctly that this is the only thing that needed to be added, I mean as far as the interface goes there are no changes between iLexer4 and iLexer5?

[Ekopalypse]

@bas-de-reuver

No, there are 3 additional methods defined by ILexer5, but it looks like they are not used yet.
However, this could become an issue in the future.

class ILexer5 : public ILexer4 {
public:
        virtual const char * SCI_METHOD GetName() = 0;
        virtual int SCI_METHOD  GetIdentifier() = 0;
        virtual const char * SCI_METHOD PropertyGet(const char *key) = 0;
};

[Shridhar Kumar]

@Ekopalypse, thanks for the hints.

Without the implementation of the 3 additional methods, Notepad++ will crash when any of these following functions are called from the PythonScript console (or, potentially its SCI_xxx equivalents are invoked by another plugin installed in Notepad++):

editor.getLexer()
editor.getLexerLanguage()
editor.setProperty('TESTING', 'Hello, World!')
editor.getProperty('TESTING')

See issue: https://github.com/BdR76/CSVLint/issues/26
Fixed in PR: https://github.com/BdR76/CSVLint/pull/27

[rdipardo]

@shridhar-kumar,
even a basic implementation of Scintilla::ILexer5 should implement every method you see here. @ozone10’s rainlexer is a good, complete example.

[rdipardo]

Fixed in PR: https://github.com/BdR76/CSVLint/pull/27

Builds and works fine. I left comments on some .NET-related stuff, but here’s a diff to sum it all up:

diff --git a/CSVLintNppPlugin/PluginInfrastructure/Lexer.cs b/CSVLintNppPlugin/PluginInfrastructure/Lexer.cs
index f636ee7..44fcee4 100644
--- a/CSVLintNppPlugin/PluginInfrastructure/Lexer.cs
+++ b/CSVLintNppPlugin/PluginInfrastructure/Lexer.cs
@@ -11,7 +11,7 @@ internal static class ILexer
     {
         public static readonly string Name = "CSVLint\0";
         public static readonly string StatusText = "CSV Linter and validator\0";
-        static readonly int LexerID = 1976;
+        public static readonly int LexerID = 1976;
 
         public static char separatorChar = ';';
         public static List<int> fixedWidths;
@@ -341,7 +341,8 @@ public static int Version(IntPtr instance)
              * ILexer5 must be provided for Scintilla version 5.0 or later.
              */
             //GC.Collect();  // test to see if the methods do get garbage collected
-            return 2;
+            // https://sourceforge.net/p/scintilla/code/ci/default/tree/include/ILexer.h#l45
+            return 3; 
         }
 
         // virtual void SCI_METHOD Release() = 0
@@ -822,14 +823,14 @@ public static int SubStylesLength(IntPtr instance, int style_base)
         public static int StyleFromSubStyle(IntPtr instance, int sub_style)
         {
             // For a sub style, return the base style, else return the argument.
-            return 0;
+            return sub_style;
         }
 
         // virtual int SCI_METHOD PrimaryStyleFromStyle(int style) = 0;
         public static int PrimaryStyleFromStyle(IntPtr instance, int style)
         {
             // For a secondary style, return the primary style, else return the argument.
-            return 0;
+            return style;
         }
 
         // virtual void SCI_METHOD FreeSubStyles() = 0;
@@ -886,7 +887,7 @@ public static IntPtr PropertyGet(IntPtr instance, IntPtr key)
             /*  returns Property Value.
              *  Equivalent to editor.getProperty('key') in PythonScript.
              */
-            return Marshal.StringToHGlobalAnsi("N/A");
+            return IntPtr.Zero;
         }
     }
 }

[Ekopalypse]

@shridhar-kumar said in Remove the external lexer support due to Scintilla 5:

editor.setProperty(‘TESTING’, ‘Hello, World!’)

Yes, that has changed in the meantime.
But are you using the latest PythonScript version?
It doesn’t crash on me, but there seems to be an issue with the property calls.

[Alan Kilborn]

@ekopalypse said in Remove the external lexer support due to Scintilla 5:

But are you using the latest PythonScript version?

You should probably state what that is.
Was it the intent of your Debug info window to show it? – it doesn’t.

[Alan Kilborn]

Here’s how I see the bug, on N++ 8.4.2(RC2):

[Shridhar Kumar]

@ekopalypse,

To clarify, I was stating that when you have:

1. CSVLint 0.4.5.1 installed
2. Have a .csv file open and in the active tab in the editor

When these two conditions are met , and then if you run editor.getProperty('Testing'), Notepad++ will indeed crash. See: https://github.com/BdR76/CSVLint/issues/25

Just the editor.setProperty('TESTING', 'Hello, World!') by itself will not cause the CSVLint 0.4.5.1 plugin to crash NPP. It is actually a setup step for running editor.getProperty('TESTING'). I am aware that one could run editor.getProperty('xxx') without actually having run a corresponding editor.setProperty('xxx'). But I put it in there lest someone may wonder if CSVLint is crashing NPP only because I didn’t run the editor.setProperty().

In your screenshot, a Python file is in view. Therefore, editor.getProperty('TESTING') works fine without any crashes.

[Shridhar Kumar]

@rdipardo said in Remove the external lexer support due to Scintilla 5:

@shridhar-kumar,
even a basic implementation of Scintilla::ILexer5 should implement every method you see [here][0]. @ozone10’s [rainlexer][1] is a good, complete example.

Just to clarify, CSVLint is not my plugin. I was only volunteering to provide a fix for it because it was causing crashes in NPP when I tried to open a .csv file while the side panel for my Fixed Width Data Visualizer (FWDataViz) plugin was also open. This started only with the 0.4.5.1 release of CSVLint. The prior version 0.4.5 worked just fine with FWDataViz in NPP 8.3.3.

I traced the crashes down to the partially complete iLexer5 upgrade changes in the CSVLint 0.4.5.1 code.

Just to clarify, FWDataViz was not affected by the Scintilla5/Lexilla5 upgrade in NPP 8.4. This is because FWDataViz uses the Scintilla styles directly instead of the lexer. So, I was confident that the source of crashes lay in the CSVLint code changes.

[Shridhar Kumar]

@alan-kilborn said in Remove the external lexer support due to Scintilla 5:

Here’s how I see the bug, on N++ 8.4.2(RC2):
…

Yes, I was planning to comment on that as well in a new conversation thread. The bug occurs only when the lexer is active for the current document. If you try the same with an unlexed (.txt file for example), the editor.getProperty('TESTING') works just fine and returns the value set earlier.

Even in the CSVLint code, you will find a bunch of editor.setProperty() calls. For example, these lines in Main.cs:

            // pass separator to lexer
            string sepchar = csvdef.Separator.ToString();
            string sepcol = Settings.SeparatorColor ? "1" : "0";
            editor.SetProperty("separator", sepchar);
            editor.SetProperty("separatorcolor", sepcol);

So, when I was working on the PR for fixing the CSVLint issue, I did try to return these property values back in the PropertyGet(key) method. But the SCI_GETPROPERTY call was surprisingly returning empty even for the key values set earlier within the plugin. Therefore, this very likely seems to be a regression with the Lexilla5 code than Notepad++.

[Ekopalypse]

@shridhar-kumar

Sorry for the confusion - yes, I misunderstood your post.