Skip to content
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

Cross-platform support for URL and file type shortcut association (Windows) #117

Merged
merged 18 commits into from
Mar 3, 2023

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Feb 15, 2023

Description

Linux and macOS already support file types and URL protocol registration via the shortcuts. On Windows, this works differently, since registration happens via the... registry :D

Also Windows doesn't really care about MIME types, so instead it deals with file extensions. There are two CMD commands for extension registration: assoc and ftype. These are convenience commands that indeed put things in the registry, I think. So we might just as well work with the registry directly, but that's always scary :D (see example here).

About URL protocols... no clue yet. I need to research. I think @aganders3 had look into this? Was it this link? Windows URI registration docs.

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

TODO

  • Support for file type registration on Windows
  • Support for URL protocol registration on Windows
  • Add tests for all platforms, including cleanup
  • Document this nicely because every platform is slightly different

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Feb 15, 2023
@jaimergp jaimergp added the type::poc indicates some proof of concept or MVP work label Feb 15, 2023
@aganders3
Copy link
Contributor

Thanks @jaimergp! I was about to ping you about helping with this... I think you're always at least one step ahead of me. This generally looks good, though I don't have a great setup for testing end-to-end.

One thing I will note is that for both Windows and Linux, registering the URL and file schemes I think the command needs to include a placeholder. For Windows the file or URL will be %1 in the command, and on Linux I believe it's %f and %u depending on whether it's a file or a URL. I think this is passed automatically as an argument on macOS.

Perhaps this should be added to the default commands, as well as documented for users specifying a different command?

@jaimergp
Copy link
Contributor Author

Perhaps this should be added to the default commands, as well as documented for users specifying a different command?

At the very least documented, yes. There's no such thing as "default command", and we can't guess where in the command the argument needs to be. We might be able to give a hint during validation but honestly I think it's reasonable to expect users to read the documentation in this advanced feature. If they don't include it they'll see it's not really working so... :D

And look at that, passing tests! :D

@aganders3
Copy link
Contributor

Ah, gotcha. I was mis-reading _process_command() to be setting a default command.

It's a nice idea to hint during validation but I agree just documenting should be enough. :)

@jaimergp
Copy link
Contributor Author

Ah, gotcha. I was mis-reading _process_command() to be setting a default command.

I wasn't at my best when naming methods and attributes. We can change anything at this stage, so feel free to suggest alternatives if you want! This library hasn't had a lot of eyes on it yet...

@aganders3
Copy link
Contributor

aganders3 commented Feb 16, 2023

I think this is passed automatically as an argument on macOS.

Sadly I was wrong about this.

I think we need to do something more in the osx_launcher to make this work. It might be simpler to write in swift or objective-c?

https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1623112-application

https://apple.stackexchange.com/questions/46666/how-to-access-a-clicked-url-in-a-url-handler-application-created-in-automator

@jaimergp
Copy link
Contributor Author

Hm, I see, so we might need to implement the protocol handling in the launcher.

It's now written in C but it already uses Apple APIs. I wonder if we can keep it in C (so it's easily compiled on conda machinery) instead of going full Apple native with Swift or Objective-C.

Some examples I found on Github (I love their new search!):

That last link shows it might be a bad idea to do it in C, even if possible... As long as we can compile it with the usual conda-forge machinery (because we will need to do it as part of the submission to the feedstock, at least once), I am ok with trying a different language. My C is not the best and actually the launcher is / was the first thing I wrote from scratch 😅

We can tackle that in a separate PR.

@jaimergp
Copy link
Contributor Author

Reading about Finicky (routes http[s] URLs to different browser/apps), I checked their delegate integration at https://github.com/johnste/finicky/blob/master/Finicky/Finicky/AppDelegate.swift

Then I thought, if we can call that Swift module from C, then we are done! So I searched and I found this thread: https://forums.swift.org/t/best-way-to-call-a-swift-function-from-c/9829/13

Which takes us to https://github.com/ingconti/CallingSwiftFromC

Which I think it's what we need? :D

We need to resolve compilation and so on, but maybe as a Framework or something? https://medium.com/@yuliiasynytsia/link-static-c-library-to-swift-framework-as-a-private-module-97eae2fec75e

To pass the URL to Python, the simplest would be to forward it via argv, but I don't know how this will work once the application is launched? How do we forward things to a running process? It would need a running service or something, plus emitters/listeners... which is not in the scope of menuinst afaik.

@jaimergp jaimergp changed the title Cross-platform support for URL and file type shortcut association (WIP) Cross-platform support for URL and file type shortcut association (Windows) Feb 22, 2023
@jaimergp jaimergp mentioned this pull request Feb 22, 2023
10 tasks
@jaimergp
Copy link
Contributor Author

jaimergp commented Mar 3, 2023

I'll merge to devel in a bit to deal with #119 separately 🚀

@jaimergp jaimergp marked this pull request as ready for review March 3, 2023 17:08
@jaimergp jaimergp requested a review from a team as a code owner March 3, 2023 17:08
@jaimergp jaimergp merged commit 0684438 into conda:cep-devel Mar 3, 2023
@jaimergp jaimergp added this to the v2.0 milestone Sep 13, 2023
@mrclary
Copy link

mrclary commented Jan 25, 2024

@jaimergp, was this PR supposed to allow file-type association with the Windows shortcut? Because that doesn't seem to be working for Spyder. Is there supposed to be a certain specification in the menu.json file other than app_user_model_id?
Nevermind, I now see file_extensions in the documentation.

@mrclary
Copy link

mrclary commented Jan 25, 2024

@jaimergp, I get a permissions error when attempting to use file_extensions:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\Users\rclary\AppData\Local\spyder-6\lib\site-packages\menuinst\utils.py", line 426, in wrapper_elevate
    return func(
  File "C:\Users\rclary\AppData\Local\spyder-6\lib\site-packages\menuinst\api.py", line 62, in install
    paths += menu_item.create()
  File "C:\Users\rclary\AppData\Local\spyder-6\lib\site-packages\menuinst\platforms\win.py", line 167, in create
    self._register_file_extensions()
  File "C:\Users\rclary\AppData\Local\spyder-6\lib\site-packages\menuinst\platforms\win.py", line 365, in _register_file_extensions
    register_file_extension(ext, identifier, command, icon=icon, mode=self.menu.mode)
  File "C:\Users\rclary\AppData\Local\spyder-6\lib\site-packages\menuinst\platforms\win_utils\registry.py", line 75, in register_file_extension
    winreg.SetValueEx(subkey, "DefaultIcon", 0, winreg.REG_SZ, icon)
PermissionError: [WinError 5] Access is denied

Should this only work for _mode="system"?

@jaimergp
Copy link
Contributor Author

Should this only work for _mode="system"?

Hm, maybe we need to add some checks here, yes. Can you open a new issue? Thanks!

mrclary added a commit to mrclary/spyder-feedstock that referenced this pull request Jan 29, 2024
Note: this currently results in a PermissionError (conda/menuinst#117 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA type::poc indicates some proof of concept or MVP work
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants