pathcopycopy icon indicating copy to clipboard operation
pathcopycopy copied to clipboard
verified publisher icon clechasseur

[BUG] Deselecting all commands from submenu actually displays all commands instead of none in the submenu

Open ulu opened this issue 2 years ago • 3 comments

Describe the bug removing all entries from submenu resulting in a garbaged submenu.

To Reproduce Steps to reproduce the behavior:

  1. go to UNC-Path and right right click on an entry

  2. click on "Path Copy > Settings..."

  3. remove all check marks

  4. set check marks for "copy long UNC path" in "Main menu" and "Submenu"

  5. click "OK" button

  6. right right click on an UNC-Path entry

  7. see a main menu entry for "copy long UNC path"

  8. right right click on an UNC-Path entry

  9. click on "Path Copy > "

  10. see a Submenu with 2 entries: "copy long UNC path" and "Settings ..."

  11. click on "Path Copy > Settings..."

  12. remove all check marks

  13. set check marks for "copy long UNC path" only in "Main menu"

  14. click "OK" button

  15. right right click on an UNC-Path entry

  16. see a main menu entry for "copy long UNC path"

  17. right right click on an UNC-Path entry

  18. click on "Path Copy > "

  19. [ERROR] see a Submenu with lots of entries

Expected behavior I want to see no submenu entries except "Settings ..." if I have only "Main menu" entries. My preferred solution is to get rid of the menu entry in context menu (explorer right click) at all. The visibility of the submenu structure in context menu (explorer right click) should be configurable. Because the settings are accessible over windows menu and I only want the Opt-In features I checked in pathcopycopy configuration.

Screenshots 01_settings 02_garbaged_submenu

Software (please complete the following information):

  • OS: Windows 10 Enterprise, 22H2
  • Path Copy Copy Version 20.0

ulu avatar Aug 01 '23 08:08 ulu

This is due to this part of the code:

https://github.com/clechasseur/pathcopycopy/blob/default/PathCopyCopy/src/PathCopyCopyContextMenuExt.cpp#L422-L423

I think this is due to a legacy behavior that should have been removed, but I'm not sure yet.

clechasseur avatar Aug 02 '23 02:08 clechasseur

Seems a good line to start. You could check it by removing the complete line from the code. But you will get a nullptr exception in line 429. So you have to guard this code with a check for a nullptr like if (pvspPlugins != nullptr) In addition I would not rely on the try-catch because it is forwarded in the catch block.

Currently I have no cpp environment installed. So I cannot help here at the moment...

ulu avatar Aug 08 '23 20:08 ulu

I think this is mostly due to legacy behavior - the way to store commands displayed in the submenu has evolved quite a lot over the years and gone through at least two major refactorings. Now, we should consider an empty list to be different from the absence of a setting - we do that for the main menu, but not for the submenu, which is a bug.

clechasseur avatar Aug 09 '23 04:08 clechasseur