From 9e53a886063e648e4a2b6b619441904da1aafbcb Mon Sep 17 00:00:00 2001
From: Marco Esters <mesters@anaconda.com>
Date: Thu, 6 Jun 2024 07:10:29 -0700
Subject: [PATCH] Only delete `Menu` when empty (#218)

* Only remove Windows start menu directory if empty

* Add install/remove test

* Remove redundant warning for terminal profile

* Only remove Linux directory entry when menu is empty

* Add news item

* Check if menu location exists before iterating

* Update menuinst/platforms/win.py

Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>

---------

Co-authored-by: jaimergp <jaimergp@users.noreply.github.com>
---
 menuinst/platforms/linux.py      |  2 +-
 menuinst/platforms/win.py        | 13 ++++++++++---
 news/218-only-delete-empty-menus | 19 +++++++++++++++++++
 tests/test_api.py                | 24 ++++++++++++++++++++++++
 4 files changed, 54 insertions(+), 4 deletions(-)
 create mode 100644 news/218-only-delete-empty-menus

diff --git a/menuinst/platforms/linux.py b/menuinst/platforms/linux.py
index ed24c519..76e99667 100644
--- a/menuinst/platforms/linux.py
+++ b/menuinst/platforms/linux.py
@@ -65,11 +65,11 @@ def create(self) -> Tuple[os.PathLike]:
         return (path,)
 
     def remove(self) -> Tuple[os.PathLike]:
-        unlink(self.directory_entry_location, missing_ok=True)
         for fn in os.listdir(self.desktop_entries_location):
             if fn.startswith(f"{self.render(self.name, slug=True)}_"):
                 # found one shortcut, so don't remove the name from menu
                 return (self.directory_entry_location,)
+        unlink(self.directory_entry_location, missing_ok=True)
         self._remove_this_menu()
         return (self.directory_entry_location,)
 
diff --git a/menuinst/platforms/win.py b/menuinst/platforms/win.py
index 2bff6390..e001e7d9 100644
--- a/menuinst/platforms/win.py
+++ b/menuinst/platforms/win.py
@@ -36,8 +36,16 @@ def create(self) -> Tuple[os.PathLike]:
         return (self.start_menu_location,)
 
     def remove(self) -> Tuple[os.PathLike]:
-        log.debug("Removing %s", self.start_menu_location)
-        shutil.rmtree(self.start_menu_location, ignore_errors=True)
+        # Only remove if the Start Menu directory is empty in case applications share a folder.
+        menu_location = Path(self.start_menu_location)
+        if menu_location.exists():
+            try:
+                # Check directory contents. If empty, it will raise StopIteration
+                # and only in that case we delete the directory.
+                next(menu_location.iterdir())
+            except StopIteration:
+                log.debug("Removing %s", self.start_menu_location)
+                shutil.rmtree(self.start_menu_location, ignore_errors=True)
         return (self.start_menu_location,)
 
     @property
@@ -344,7 +352,6 @@ def _add_remove_windows_terminal_profile(self, location: Path, remove: bool = Fa
 
         if remove:
             if index < 0:
-                log.warning(f"Could not find terminal profile for {name}.")
                 return
             del settings["profiles"]["list"][index]
         else:
diff --git a/news/218-only-delete-empty-menus b/news/218-only-delete-empty-menus
new file mode 100644
index 00000000..39b51929
--- /dev/null
+++ b/news/218-only-delete-empty-menus
@@ -0,0 +1,19 @@
+### Enhancements
+
+* <news item>
+
+### Bug fixes
+
+* Only delete menus when they do not contain any items. (#218)
+
+### Deprecations
+
+* <news item>
+
+### Docs
+
+* <news item>
+
+### Other
+
+* <news item>
diff --git a/tests/test_api.py b/tests/test_api.py
index 495b7bd0..1e4c300a 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -145,6 +145,30 @@ def test_install_prefix(delete_files):
     check_output_from_shortcut(delete_files, "sys-prefix.json", expected_output=sys.prefix)
 
 
+def test_install_remove(tmp_path, delete_files):
+    metadata = DATA / "jsons" / "sys-prefix.json"
+    (tmp_path / ".nonadmin").touch()
+    paths = set(install(metadata, target_prefix=tmp_path, base_prefix=tmp_path, _mode="user"))
+    delete_files.extend(paths)
+    files_found = set(filter(lambda x: x.exists(), paths))
+    assert files_found == paths
+    if PLATFORM != "osx":
+        metadata_2 = json.loads(metadata.read_text())
+        metadata_2["menu_items"][0]["name"] = "Sys.Prefix.2"
+        paths_2 = set(
+            install(metadata_2, target_prefix=tmp_path, base_prefix=tmp_path, _mode="user")
+        )
+        delete_files.extend(paths_2)
+        files_found = set(filter(lambda x: x.exists(), paths_2.union(paths)))
+        assert files_found == paths_2.union(paths)
+        remove(metadata_2, target_prefix=tmp_path, base_prefix=tmp_path, _mode="user")
+        files_found = set(filter(lambda x: x.exists(), paths_2.union(paths)))
+        assert files_found == paths
+    remove(metadata, target_prefix=tmp_path, base_prefix=tmp_path, _mode="user")
+    files_found = set(filter(lambda x: x.exists(), paths))
+    assert files_found == set()
+
+
 def test_overwrite_existing_shortcuts(delete_files, caplog):
     """Test that overwriting shortcuts works without errors by running installation twice."""
     check_output_from_shortcut(