-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve errors and warnings when shortcuts already exist #205
Conversation
I wonder now if we should rename existing shortcuts so there's a backup in case we overstepped accidentally. |
Any thoughts about this question? |
I think this would be too drastic of a change in behavior and can quickly lead to cluttering up a user's system if the designer of the menu item is not careful. |
I was thinking about how the macos case is a bit different. On Windows and Unix, the "shortcut file" is just a file whose sole purpose is metadata to launch a program. However in macOS it is a directory, and we might be very unlucky if someone publishes a shortcut named I think that at the very least we should send it to Trash (via osascript maybe?), instead of removing it. At least they'll have the opportunity for recovery. The annoyance of having multiple copies is not as worse as deleting an important directory. |
This is not a risk for system-critical applications because they are in
The problem isn't just that they don't exist, but that I see the following ways out:
|
None is ideal... I'm leaning towards (3), but we should check something first. Does the name of the .app directory affect the display name in the UI? Because it does not (and it gets it from the Plist config inside), maybe we can just add some timestamp to the filename to avoid collisions. |
Wrt to (2), I'm not sure it's an antipattern. I do see a "Chrome apps" subdirectory in my |
I generally wouldn't want timestamps because they don't tell the user anything. However, the point is moot since there is a display change: the name next to the Apple symbol in the taskbar shows the name of the file, not the name inside
While it may not be an anti-pattern, it will at least be a breaking change for other developers who have already pushed MacOS apps using For now, a better error message may indeed be the solution. |
docs/source/defining-shortcuts.md
Outdated
@@ -18,6 +18,11 @@ four keys | |||
- `osx`- see {class}`MacOS schema <menuinst._schema.MacOS>` | |||
- `win`- see {class}`Windows schema <menuinst._schema.Windows>` | |||
|
|||
```{warning} | |||
`menuinst` will overwrite existing shortcuts, so `menu_name` and `name` must be chosen accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to update this to cover the macOS logic.
Co-authored-by: jaimergp <[email protected]>
Description
On Windows and Linux, shortcuts are overwritten when they already exist. On MacOS, menuinst exits with an error because it cannot create the Resources directory inside the app.
Since an app is not a simple file, removing the entire directory before creating the new app is the easiest way to ensure that a proper overwrite was done. The test I fails with the current menuinst.I also decided to add warnings to when shortcuts are overwritten. This behavior has always been a little hidden and could cause some surprises if users are not aware of this behavior.
EDIT: after some discussion, a more prudent way is to improve the error message. Individual packages can still remove the app using pre-link features if they want to overwrite.
Closes #203.
Checklist - did you ...
news
directory (using the template) for the next release's release notes?