search

notepad-plus-plus / nppShell

Provide Explorer context menu entry "Edit with Notepad++"
GNU General Public License v3.0
25 stars 13 forks source link

More logging #51

Closed GurliGebis closed 11 months ago

GurliGebis commented 11 months ago

This adds more logging, to help debug issues with the context menu

GurliGebis commented 11 months ago

@donho this adds more logging and fixes a potential race condition. Can you merge it, and create a test installer, including the new DLL, to help debug?

donho commented 11 months ago

@GurliGebis no problem to create the Installer, but could we let user test it before then there might be more evolution in this PR?

donho commented 11 months ago

@GurliGebis Here are signed installer binaries which contain NppShell of this PR: http://download.notepad-plus-plus.org/repository/MISC/nppShell_more_log/

GurliGebis commented 11 months ago

@donho thanks, I'll use it to debug some more. You might want to merge it, since it adds more debugging, and fixes a race condition.

donho commented 11 months ago

@GurliGebis

You might want to merge it, since it adds more debugging, and fixes a race condition.

Merged. Thank you for the PR. Please thinking of clean up the logs, once the issues are fixed.

GurliGebis commented 11 months ago

@donho since it is disabled normally, I don't think removing it is needed?

donho commented 11 months ago

@GurliGebis If the issue is fixed, it means the log is no more needed. For the sake of facility of maintenance, it's always good to clean up the code. Don't you think so?

GurliGebis commented 11 months ago

@donho Keeping it would make sense for this, since it would be easy to have people enable it later on, if other issues with this shows up. I just there was a way to ensure that there modern command is also shown in the classic menu, it would be great - but people seems to mess around with stuff way too much for that to be stable (like trying to remove the modern right click menu using registry hacks).

donho commented 11 months ago

@GurliGebis OK. I see your point. Let it be then. Thank you for your explanation!