x64 changes

[oleg-shilo]

First of all I want to congratulate you on the fantastic product that you developed and generously shared with the developers community.

Your NotepadPlusPlusPluginPack.Net framework is in fact the only working portable solution that can be used today with Notepad++. By providing a .NET bridge for Notepad++ you are in fact fixing the conceptual problem the Notepad++ has - non-portable hosting runtime. Despite not being publicly discussed/recognized, this problem is in fact quite severe as all other popular professional editors avoided making this mistake by opting to dynamic languages for their hosting runtime API.

This PR contain very minor code changes to allow x64 complete support.

Some of the changes despite being simple, are quite essential as they (if not fixed) prevent x64 hosting in 100% cases if certain API is used (e.g. GetOpenFiles).

Please be aware that my manually inserted code may interfere with your auto-generated content in *gateway classes.

Please also note that I deliberately didn't touch your exporting routine as I am not sure about your plans for it. But if VS2017 is to be supported then the *.targets will also need to be changed:

• LibToolPath needs to be completely removed
• Project target should be set to .NET 4.0

I have also created a few convenient routines for an easy access to the underlying API: Npp.Editor and Npp.GetCurrentDocument()

---

Extending NotepadPlusPlusPluginPack.Net

I also wanted to discuss with you an alternative authoring/deployment model that I used in my CS-Script.Npp plugin. This model is based on your NotepadPlusPlusPluginPack.Net framework and I would like to make this additional authoring approach publicly available one or another way.

You can have a look at it from my GitHub page: https://github.com/oleg-shilo/cs-script.npp/blob/master/src/NppPlugin.Host/NppPlugin.Host.7z. The included (in the attachment) readme.MD file contains detailed explanations on what it does and how to use it.

To be honest this alternative authoring model is not fully consistent with you original solution and I am not sure how it can be merged with your codebase. Thus it may make sense to publish it as a standalone GitHub project. But I didn't want to do this without talking you first.

Thus please consider these options:

• You can take the attached solution and find the way incorporating it into the NotepadPlusPlusPluginPack.Net
• I can create a dedicated GitHub project and publish NppPlugin.Host solution as a derivative of your NotepadPlusPlusPluginPack.Net. With the full credit for the underlying framework given to you.

Please let me know your thoughts.

[jokedst]

I like the NotepadPPGateway additions, and your plugin CS-Script is seriously awesome! But could you please show what the "very minor code changes to allow x64 complete support" are? This PR contains so much reformatting that it's hard to see what's actually changed :P

[kbilsted]

Hi Oleg

Thanks for the PR and your kind words. While I have put in significant time into the plugin framework I am building on top of others :-) And specifically the dll merging/processing is not mine.

The generated files are essential since notepad++ and scintilla keep changing their interfaces. Hence we need an approach that supports this.

Also we have to consider this pr breaks for vs2015 and I'm a bit unsure if the stability of x64 notepad++ releases. Many of them have been limiting for me so I've stuck with x32. But I guess both concerns are a bit dated by now.

[oleg-shilo]

OK then. I understand your concerns. Then I after finishing with CS-Script plugin x64 migration and will release a standalone NppPlugin.Host package as a derivative from your project.

Now, abut the PR. The difficulties with tracking the changes are due to the PR code being formatted/normalized with CodeMaid and your original code in many cases being auto-generated. I am not sure how to permanently reconcile these different formatting policies so probably it makes sense just to close this PR and process the changes manually. Fortunately they are minor.

I have attached the zip file with both (yours and mine) versions of PluginInfrastructure folder being CodeMaid normalized. The differences are very easy to follow with any good diff utility (e.g. kdif3):

Please note that all the changes can be grouped into two categories: improvements and critical fixes.

I have also manually described the critical fixes in the enclosed critical_changes.txt file.

This fixes are vital as the allow your code to be valid under x64. Also note that none of the changes break x86. So you are good.

As for VS2017 vs 2015, I found that after re-targeting the project for .NET v4.0 and removing LibToolPath from *targets. Your code base works well on both VS2015 and VS2017 and x86 and x64.

My point is that you are in fact completely ready for both versions VS and CPU types. The only thing you need to do is to bring these "critical fixes" and adjust the *targets file.

Taking or not taking my improvements changes is entirely up to you.

Cheers, Oleg

PluginInfrastructure.zip

[jokedst]

Thanks for the effort! Though think something went wrong, because critical_changes.txt is empty :(

[kbilsted]

your work is by no means lost. I think we may find a way to generate better code than we do now. I know there are problems with the conversion from eg IntPrt to int and others but I've had a hard time getting my plugins into the official channels, as well as this plugin pack - so I've prioritized other things for a while. I really hope you will continue contributing to this project :-) Before spending too much time, lets first discuss ideas and coding approaches. Like with the importance of generating the code in the plugin pack.. it found so many missing bindings when I created it.

the key to getting things through is to do small PR's focusing on one thing. Eg. dont mit x64 with color issues. I know its more tedious, but it makes everything so much easier

[oleg-shilo]

Not a problem. This is the content of the missing description file"

NotepadPPGateway.cs

- Win32.SendMessage(PluginBase.nppData._nppHandle, (uint)NppMsg.NPPM_GETFULLCURRENTPATH, 0, path)
+ Win32.SendMessage(PluginBase.nppData._nppHandle, (uint)NppMsg.NPPM_GETFULLCURRENTPATH, path.Capacity, path);

ClikeStringArray.cs

- public ClikeStringArray(int num, int stringCapacity)
        {
            _nativeArray = Marshal.AllocHGlobal((num + 1) * IntPtr.Size);
            _nativeItems = new List<IntPtr>();
            for (int i = 0; i < num; i++)
            {
                IntPtr item = Marshal.AllocHGlobal(stringCapacity);
                Marshal.WriteIntPtr((IntPtr)((int)_nativeArray + (i * IntPtr.Size)), item);
                _nativeItems.Add(item);
            }
            Marshal.WriteIntPtr((IntPtr)((int)_nativeArray + (num * IntPtr.Size)), IntPtr.Zero);
        }

        public ClikeStringArray(List<string> lstStrings)
        {
            _nativeArray = Marshal.AllocHGlobal((lstStrings.Count + 1) * IntPtr.Size);
            _nativeItems = new List<IntPtr>();
            for (int i = 0; i < lstStrings.Count; i++)
            {
                IntPtr item = Marshal.StringToHGlobalUni(lstStrings[i]);
                Marshal.WriteIntPtr((IntPtr)((int)_nativeArray + (i * IntPtr.Size)), item);
                _nativeItems.Add(item);
            }
            Marshal.WriteIntPtr((IntPtr)((int)_nativeArray + (lstStrings.Count * IntPtr.Size)), IntPtr.Zero);
        }

+ public ClikeStringArray(int num, int stringCapacity)
        {
            _nativeArray = Marshal.AllocHGlobal((num + 1) * IntPtr.Size);
            _nativeItems = new List<IntPtr>();
            for (int i = 0; i < num; i++)
            {
                IntPtr item = Marshal.AllocHGlobal(stringCapacity);
                Marshal.WriteIntPtr(_nativeArray + (i * IntPtr.Size), item);
                _nativeItems.Add(item);
            }
            Marshal.WriteIntPtr(_nativeArray + (num * IntPtr.Size), IntPtr.Zero);
        }

        public ClikeStringArray(List<string> lstStrings)
        {
            _nativeArray = Marshal.AllocHGlobal((lstStrings.Count + 1) * IntPtr.Size);
            _nativeItems = new List<IntPtr>();
            for (int i = 0; i < lstStrings.Count; i++)
            {
                IntPtr item = Marshal.StringToHGlobalUni(lstStrings[i]);
                Marshal.WriteIntPtr(_nativeArray + (i * IntPtr.Size), item);
                _nativeItems.Add(item);
            }
            Marshal.WriteIntPtr(_nativeArray + (lstStrings.Count * IntPtr.Size), IntPtr.Zero);
        }

And just today I found another problem that manifests itself in both x86 and x64. ClikeStringArray.Dispose must have try...catch. Otherwise it brings down the whole CLR and N++ crashes. After various test I found the scenario when this happens 100%. I am also under impression concluded that it is not necessarily due to the managed interface (your adapter) but probably because of some other N++ plugins being "playful" with the unmanaged memory.

In any case any unhandled exceptions in finalizers are extremely severe.

[kbilsted]

@mahee96 how much of this PR is left to merge over after your changes?

[mahee96]

@kbilsted I will look into this PR with my test branch and will get back once I see major changes that needs to be merged.

[mahee96]

https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.marshal.writeintptr?view=netcore-3.1#System_Runtime_InteropServices_Marshal_WriteIntPtr_System_IntPtr_System_IntPtr_

Shows that there is no need of cast for a pointer which is already IntPtr type and for pointer arithmetic IntPtr.Size property can be used to calculate offset for iterating.

[kbilsted]

Btw this was exhaustive 😅

if you swing by I'll serve you a beer :-) @mahee96

[mahee96]

@kbilsted now that you merged PR #58 , I think it is safe to close this PR which targets the same thing.

[mahee96]

closing the issue as indicated above.