Skip to content

Conversation

@GurliGebis
Copy link
Contributor

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

@GurliGebis
Copy link
Contributor Author

@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
Copy link
Member

donho commented Oct 17, 2023

@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
Copy link
Member

donho commented Oct 17, 2023

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

@GurliGebis
Copy link
Contributor Author

@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 donho closed this in 74affad Nov 5, 2023
@donho
Copy link
Member

donho commented Nov 5, 2023

@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
Copy link
Contributor Author

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

@donho
Copy link
Member

donho commented Nov 6, 2023

@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
Copy link
Contributor Author

@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).

@GurliGebis GurliGebis deleted the more-logging branch November 6, 2023 09:16
@donho
Copy link
Member

donho commented Nov 6, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants