jokedst / CsvQuery

Plugin for Notepad++ that treats CSV files as (read only) SQL tables
153 stars 28 forks source link

Latest Notepad++ 8.2.2 dev builds crash #33

Closed vinsworldcom closed 2 years ago

vinsworldcom commented 2 years ago

Using the latest Notepad 8.2.2 development build and opening the CSV Query window causes Notepad++ to crash. See this thread for info on other plugins that are affected as well.

It may be as simple as recompiling with the latest headers from Notepad++ project:

Cheers.

cmbsolutions commented 2 years ago

Same goes for Notepad++ 8.3 x64

vinsworldcom commented 2 years ago

Same goes for Notepad++ 8.3 x64

Yes, 8.2.2 development became 8.3 release. This update allows larger files thus API changes to deal with int become intptr_t

rdipardo commented 2 years ago

To their credit, the maintainers of NppPlugin.NET have been (slowly) trying to keep up with the evolution of Scintilla's interface: https://github.com/kbilsted/NotepadPlusPlusPluginPack.Net/pull/75

Unfortunately a drop-in replacement won't come fast enough. Here's a quick (untested) patch that might be good enough:

diff --git a/CsvQuery/PluginInfrastructure/GatewayDomain.cs b/CsvQuery/PluginInfrastructure/GatewayDomain.cs
index df9cf5b..ecbac0c 100644
--- a/CsvQuery/PluginInfrastructure/GatewayDomain.cs
+++ b/CsvQuery/PluginInfrastructure/GatewayDomain.cs
@@ -175,9 +175,9 @@ namespace CsvQuery.PluginInfrastructure
     [StructLayout(LayoutKind.Sequential)]
     public struct CharacterRange
     {
-        public CharacterRange(int cpmin, int cpmax) { cpMin = cpmin; cpMax = cpmax; }
-        public int cpMin;
-        public int cpMax;
+        public CharacterRange(IntPtr cpmin, IntPtr cpmax) { cpMin = cpmin; cpMax = cpmax; }
+        public IntPtr cpMin;
+        public IntPtr cpMax;
     }

     public class Cells
@@ -196,18 +196,18 @@ namespace CsvQuery.PluginInfrastructure
         IntPtr _ptrSciTextRange;
         bool _disposed = false;

-        public TextRange(CharacterRange chrRange, int stringCapacity)
+        public TextRange(CharacterRange chrRange, long stringCapacity)
             : this(chrRange.cpMin, chrRange.cpMax, stringCapacity)
         { }

-        public TextRange(int cpmin, int cpmax, int stringCapacity = 0)
+        public TextRange(IntPtr cpmin, IntPtr cpmax, long stringCapacity = 0)
         {
             // The capacity must be _at least_ the given range plus one
-            stringCapacity = Math.Max(stringCapacity, Math.Abs(cpmax - cpmin) + 1);
+            stringCapacity = Math.Max(stringCapacity, Math.Abs(cpmax.ToInt64() - cpmin.ToInt64()) + 1);

             _sciTextRange.chrg.cpMin = cpmin;
             _sciTextRange.chrg.cpMax = cpmax;
-            _sciTextRange.lpstrText = Marshal.AllocHGlobal(stringCapacity);
+            _sciTextRange.lpstrText = Marshal.AllocHGlobal(new IntPtr(stringCapacity));
         }

         [StructLayout(LayoutKind.Sequential)]
diff --git a/CsvQuery/PluginInfrastructure/ScintillaGateway.cs b/CsvQuery/PluginInfrastructure/ScintillaGateway.cs
index df169a7..763ce04 100644
--- a/CsvQuery/PluginInfrastructure/ScintillaGateway.cs
+++ b/CsvQuery/PluginInfrastructure/ScintillaGateway.cs
@@ -75,7 +75,7 @@ namespace CsvQuery.PluginInfrastructure
         public string GetTextRange(int start, int end)
         {
             var codepage = GetCodePage();
-            using (var tr = new TextRange(start, end))
+            using (var tr = new TextRange(new IntPtr(start), new IntPtr(end)))
             {
                 GetTextRange(tr);
                 if (codepage == (int) SciMsg.SC_CP_UTF8)
diff --git a/CsvQuery/PluginInfrastructure/Scintilla_iface.cs b/CsvQuery/PluginInfrastructure/Scintilla_iface.cs
index 32e09ab..f012661 100644
--- a/CsvQuery/PluginInfrastructure/Scintilla_iface.cs
+++ b/CsvQuery/PluginInfrastructure/Scintilla_iface.cs
@@ -3010,7 +3010,7 @@ namespace CsvQuery.PluginInfrastructure
         /// <param name="cpmin">range to search</param>
         /// <param name="cpmax">range to search</param>
         /// <param name="searchText">the search pattern</param>
-        public TextToFind(int cpmin, int cpmax, string searchText)
+        public TextToFind(IntPtr cpmin, IntPtr cpmax, string searchText)
         {
             _sciTextToFind.chrg.cpMin = cpmin;
             _sciTextToFind.chrg.cpMax = cpmax;
jokedst commented 2 years ago

Thanks! That seems to work on 8.3 It crashes 8.1 though, so I'll have to make some fixes to make sure it works in both...

vinsworldcom commented 2 years ago

It crashes 8.1 though, so I'll have to make some fixes to make sure it works in both...

I'm not an expert at this, certainly not C#, but I'm not sure that's possible. I tested my updated C++ plugin on version 8.1 and it crashed, but it works fine on 8.3. I think the issue is in changing that int to intptr_t - it's either one or the other, the intptr_t doesn't make it backward compatible with older versions of N++ that are still compiled with int.

Your new version will work for Notepad++ >=8.3.

Looking forward to testing CsvQuery!

Cheers.

rdipardo commented 2 years ago

@vinsworldcom is right.

The Scintilla library that was linked with 8.1 still expects a Sci_PositonCR to be sizeof(long) on all architectures.

jokedst commented 2 years ago

I should be able to just check the N++ version and have two data structures - one for legacy and one for 8.3+ That's what I did when the icon code changed in N++

vinsworldcom commented 2 years ago

@jokedst Do you have a test build I can try out for Notepad++ >= 8.3?

wsuhoey commented 2 years ago

This is still crashing in Notepad++ v8.3.3 (64-bit).

vinsworldcom commented 2 years ago

@wsuhoey it will continue to crash until @jokedst posts a new release addressing this issue.

vinsworldcom commented 2 years ago

@wsuhoey @cmbsolutions ~I was able to compile (through Github Actions) the latest code and called it 1.2.9 RC 1 available here for manual install if you can't live without this plugin (as I could no longer). It is working for me with the latest Notepad++ 8.4 release candidate.~

Cheers.

jokedst commented 2 years ago

Ok ok ok, sorry, been busy. I've created a 1.2.9 release that works, I'll start the process of getting it approved for the plugin manager.