pnedev / comparePlus

Compare plugin for Notepad++
GNU General Public License v3.0
985 stars 140 forks source link

Latest test build of Notepad++ crashed when Compare #275

Closed vinsworldcom closed 2 years ago

vinsworldcom commented 2 years ago

Using the latest test builds of Notepad++ where greater than 2Gb file editing is trying to be enabled, comparing regular small files fails with a crash.

1) open 2 small files 2) Plugins => Compare => Compare 3) Notepad++ crashes

Notepad++ v8.2.1 (64-bit) [NOTE THIS IS USING COMMIT] Build time : Jan 29 2022 - 14:26:54 Path : C:\usr\bin\npp64\notepad++.exe Command Line : Admin mode : OFF Local Conf mode : ON Cloud Config : OFF OS Name : Windows 10 Enterprise (64-bit) OS Version : 2009 OS Build : 19042.1466 Current ANSI codepage : 1252 Plugins : AutoSave.dll BetterMultiSelection.dll ChangedLines.dll CodeAlignmentNpp.dll ComparePlugin.dll CustomizeToolbar.dll Explorer.dll GitSCM.dll JSMinNPP.dll MarkdownPanel.dll NppExec.dll NppFTP.dll NppMenuSearch.dll PythonScript.dll QuickText.dll SurroundSelection.dll TagLEET.dll XMLTools.dll ZoomDisabler.dll

vinsworldcom commented 2 years ago

Seems like an easy fix, just add the latest from Notepad++ repo:

To your repo at:

And recompile. I'm using that experiment test compiled version I did locally with VS2019 an it's working fine so far!

Cheers.

pryrt commented 2 years ago

FYI: per this and this, with v8.2.2 RC5, the NPP installer is actively disabling ComparePlugin for anyone who has it already installed, and that might mean he's also removed it from Plugins Admin (though I'm not sure) -- which means that users are going to have to start fighting against Don unless an update-with-patch is released ASAP (preferably before v8.2.2 is fully released)

pnedev commented 2 years ago

@vinsworldcom , @pryrt ,

Michael, Peter, thanks for the heads-up :+1: I will find some time today/tonight to make a fix release to ComparePlugin (although I have again started working on Compare Plus after nearly two years and I hope to be able to release it in a month or so). The fix might require more than just changing the Scintilla headers if the change is connected with text ranges as it seems.

vinsworldcom commented 2 years ago

@pnedev maybe, but looking here

https://community.notepad-plus-plus.org/topic/22471/recompile-your-x64-plugins-with-new-header

That's all @donho mentions. I did a quick recompile of the compare plug-in with just the scintilla.h and sci_position.h files updated and it seems to work okay without crashing for me.

Also of note, I didn't realize pulling the main branch of your git repo would get me ComparePlus. So that's the one I did originally and that seemed to work fine as well with just those header file changes. Granted I only did some simple compares with both ComparePlus and compare plugin (my newly compiled versions), but neither of them crashed Notepad++ as the previous version of compare-plugin did.

Cheers.

Yaron10 commented 2 years ago

Hello Pavel,

I have again started working on Compare Plus after nearly two years and I hope to be able to release it in a month or so

👍 I'll be looking forward to that. :) Thank you and best of luck.

May I ask if adapting the current ComparePlus to NPP 64-bit would require much more work? I've been using NPP 32-bit on Win 10 64-bit because I remember you've recommended not to use ComparePlus 64-bit. Just asking. I don't mean to rush you or change your plans.

BR

vinsworldcom commented 2 years ago

@Yaron10

From my comment above you can see in my haste, I replaced the headers and compiled ComparePlus and was using it on 64-bit Notepad++ development (8.2.2) and for the simple compares I was doing - it worked fine.

Cheers.

Yaron10 commented 2 years ago

@vinsworldcom,

Thank you for the info.

Pavel recommended not to use ComparePlus 64-bit about two years ago. :) So it's not only the recent NPP changes.

pnedev commented 2 years ago

Thanks @vinsworldcom . I suspect that 64bit ComparePlus from the master branch will not scroll synchronously in vertical direction during compare. That's why I advised @Yaron10 to use only the 32bit version back then.

Hi @Yaron10 ,

I hope you are doing well :)

It will be difficult at the moment to provide 64bit fix to ComparePlus in master. I really hope to be ready in a month with the first release, thanks for understanding.

BR

Yaron10 commented 2 years ago

Hello Pavel,

I hope you are doing well too. :)

No problem at all. At the time I used Win 7 32-bit, and I didn't really know what was the issue with ComparePlus 64-bit.

I'm glad you've found some time to Compare. Thanks again. Much appreciated.

BR

pnedev commented 2 years ago

@vinsworldcom , @pryrt ,

The new release 2.0.2 is ready, I have made a PR to NppPluginList.

Thanks once again!

Yaron10 commented 2 years ago

Hello Pavel,

👍 Thanks again for your work. Highly appreciated.

BR

pnedev commented 2 years ago

Hello Yaron,

Thank you.

BR

vinsworldcom commented 2 years ago

@pnedev just tested - works great! Thank you for the quick turn around and continuing to support excellent (and in my opinion - "essential") N++ plugins.

pnedev commented 2 years ago

@vinsworldcom , Thank you too for filing the issue and for the heads-up! :+1:

vinsworldcom commented 2 years ago

I suspect that 64bit ComparePlus from the master branch will not scroll synchronously in vertical direction during compare.

Seems to be working fine for me. I'll probably keep ComparePlugin installed, but not active and use ComparePlus (with my simple header changes) for a while to get used to it. I'll let you know if anything is amiss.

Also note I'm not comparing files greater than 10Mb on the regular so will probably never hit any >2Gb specific issues dealing with text ranges that may not yet be fixed in the ComparePlus version.

Thanks for you work on this - it's really invaluable, not just for side-by-side diffs of regular files, but the "Git Diff" I use during most Notepad++ coding sessions. Better than my GitSCM plugin that relies on external TortoiseGit to give the visual side-by-side diff.

Cheers.

pnedev commented 2 years ago

Thank you @vinsworldcom 👍

Yaron10 commented 2 years ago

Hello Pavel,

The break has been relatively short this time. :) I hope you're doing well.

Thank you very much for https://github.com/pnedev/compare-plugin/commit/6b7bf7f6abc60d84bcc7130a37a50c19f53e0d64. Again, I appreciate your time and work. 👍

I've been using the 64-bit version for the last couple of months. The only issue (unique to 64-bit) I've encountered was the wrong file getting focus after clearing compare. This is fixed now.


@vinsworldcom,

You can update. Isn't ComparePlus great?

pnedev commented 2 years ago

Hello Yaron,

Nice 'hearing' from you again, I'm OK, thank you. :)

The break has been relatively short this time. :)

Don't even mention it ;)))

I would recommend using the temporary build https://github.com/pnedev/compare-plugin/commit/3f412a13eb4eba3c2e6379ead962b91ad63bd904 which uses multiple threads in changes detection phase speeding up compares with many changed sections. Here is also the 64-bit DLL

BR, Pavel

Yaron10 commented 2 years ago

Hello Pavel,

I'm glad you're well. Communicating with you is always a pleasure.

I've updated to https://github.com/pnedev/compare-plugin/commit/3f412a13eb4eba3c2e6379ead962b91ad63bd904. Thank you for this improvement as well. 👍

BR

vinsworldcom commented 2 years ago

@pnedev I've been using ComparePlus since the Notepad++ 8.2.2 / 8.3 plugin breaking change and have had no issues. Much better than the original! I've updated to the latest version you recommend and will continue to use in my daily workflow.

Should also mention I'm using the latest Notepad++ 8.3.3 commit including the Scintilla 5 upgrade: image

Working well so far!

Thank you for continued improvements to this excellent Notepad++ plugin!

Cheers.