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

Feature add install script for Windows #77

Merged
merged 5 commits into from
Mar 4, 2022

Conversation

DanSM-5
Copy link
Contributor

@DanSM-5 DanSM-5 commented Mar 3, 2022

Windows installation script.

This should make installation a smooth process for windows users.
I made it to behave as the bash installation scripts.

Example:
> .\install.ps1 firefox
Will add the registry key for ff2mpv in the NativeMessagingHost for firefox.

Featured browsers

  • Firefox
  • Chrome
  • Chromium
  • Edge

Other features

  • Allow custom installation for chromium based browsers
  • Detect python command and update it in the batch script
  • Generate a different JSON file for chromium browsers to avoid conflicts with firefox

@woodruffw
Copy link
Owner

Thanks for the PR!

Is there any functional overlap between this script and check-config-win.py? I believe another Windows user contributed that earlier script to manage the registry settings for ff2mpv.

Assuming there is some overlap, I'm not opposed to deleting that old script and completely replacing it with your work (especially since the old one doesn't support anything but Firefox).

@DanSM-5
Copy link
Contributor Author

DanSM-5 commented Mar 4, 2022

The python script does 3 things

  • checks that mpv and the registry are in place
  • add the registry key for firefox
  • remove the registry key for firefox

It mainly overlaps with the installation part but only for firefox as the powershell script supports more browsers and will generate the JSON's for chromium based browsers.

I didn't add a command to just check the configuration (like checking for mpv). I added a check for python mainly because in windows this may vary and the batch script won't work unless it uses the right command in the specific system. It would be simple add a check for MPV and just log a message if it is not found. Would this be desired?

Finally, the powershell script only does installations but it won't add the registry key gain if it is already present or it will update the value if you move the location of the repository. I could later implement a flag to allow removing the key. Other changes are local to the repository (like creating a JSON or modifying the bat script) and follow the same conventions as the sh scrips to keep it consistent.

@DanSM-5
Copy link
Contributor Author

DanSM-5 commented Mar 4, 2022

I made an update so the powershell script:

  • Add flag -uninstall to remove the ff2mpv key from the registry
  • Add flag -help to show more information about the command to call the script
  • Add a warning message in the install to notify the user that mpv could not be found

I believe that now the python script can be replaced and all its functions should be cover.

@woodruffw
Copy link
Owner

I believe that now the python script can be replaced and all its functions should be cover.

Thank you! Would you mind deleting the Python script as part of this changeset?

I'll do a final review pass tonight as well, but this LGTM overall.

@DanSM-5
Copy link
Contributor Author

DanSM-5 commented Mar 4, 2022

Done. While at it I added option to display help or exit the prompt to make it more user friendly.

@woodruffw
Copy link
Owner

CI failure is caused by the deletion. Feel free to delete that line from the CI if you have a moment, otherwise I'll clean it up post-merge.

@DanSM-5
Copy link
Contributor Author

DanSM-5 commented Mar 4, 2022

No worries, I found where the configuration was. I removed the check for the script in ci.yml.

@woodruffw woodruffw merged commit ffd980a into woodruffw:master Mar 4, 2022
@woodruffw
Copy link
Owner

Thanks again!

@DanSM-5
Copy link
Contributor Author

DanSM-5 commented Mar 5, 2022

I'll update the wiki over the weekend with usage about the new installation script on Windows.

@woodruffw
Copy link
Owner

woodruffw commented Mar 5, 2022 via email

@DanSM-5 DanSM-5 deleted the feature_install_script_windows branch July 28, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants