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 <[email protected]> --------- Co-authored-by: jaimergp <[email protected]>
conda · Jun 6, 2024 · 9e53a88 · 9e53a88
1 parent 26deb4d
commit 9e53a88
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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(