-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add file associations for Windows applications #62
Conversation
Still working on it, will check off all the boxes. |
To test the functionality of this PR, add this to your project settings in your [tool.briefcase.app.your_app_name]
template = "https://github.com/davidfokkema/briefcase-windows-app-template"
template_branch = "file-associations" |
@davidfokkema I did a quick test here and it's failing, apparently because of the directory separator character's discrepancy:
|
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.
Looks good; a couple of comments inline about improvements to MIME and icon handling, plus one escaping issue.
The directory separator was not the issue. Currently, the icon needs to be specified relative to the sources module, in your case 'frescobaldi'. This diff of [tool.briefcase.app.frescobaldi.document_type.ly]
description = "LilyPond file"
extension = "ly"
-icon = "frescobaldi/icons/org.frescobaldi.Frescobaldi"
+icon = "icons/org.frescobaldi.Frescobaldi"
url = "https://lilypond.org/"
[tool.briefcase.app.frescobaldi.document_type.ily]
description = "LilyPond included file"
extension = "ily"
-icon = "frescobaldi/icons/org.frescobaldi.Frescobaldi"
+icon = "icons/org.frescobaldi.Frescobaldi2"
url = "https://lilypond.org/" However, it complained that a GUID is reused. On a hunch, since my test repo uses two different icons for the two document types, I copied the icon and named it |
Mind that specifying the location of the icon this way will probably change after I implemented @freakboy3742's suggestions. |
@fedelibre with the current changes to the template, copying the same icon into the build directory, there is no longer any need to change your |
Thanks, I've rerun the job and the build completed successfully. I'll test the .msi file on Windows when I have time. |
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.
Looks great - and works great in my testing as well. 2 comments inline, one of which comes with a fix that I'd like someone other than me to validate works - but if it does, I think this is good to go!
Co-authored-by: Russell Keith-Magee <[email protected]>
The |
When this is merged, how would I best go about including it in the visualstudio template? As a single commit including the same changes by copying over the new files? Or somehow pulling in the different commits? |
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.
Awesome - thanks for these fixes!
Regarding the VisualStudio analog - a single commit making this set of change on that repo would be great.
One final minor housekeeping note - I've tweaked the PR description slightly, because the text "Fixes #xxx on windows" contains the magic words "Fixes #X", which Github will use to close the referenced ticket - the human readable qualifier won't be taken into consideration. In this case the issue won't be completely fixed, so we need to use "Refs #xxx" instead.
Darn, didn't think of that. Thanks for the housekeeping. And for accepting my PR, of course! |
Copied changes from beeware/briefcase-windows-app-template#62
This PR adds file associations to Windows apps. The support in briefcase.toml is already there and the feature is documented. It wasn't yet implemented for Windows, however.
Refs beeware/briefcase#1706 (corrects the problem for Windows).
I tested with https://github.com/davidfokkema/helloworld. This repository may be removed when the PR is merged. The relevant sections of
pyproject.toml
are:with the
resources
folder containing the icons inside the module. So, relative to the repository root, the icons are located insrc/helloworld/resources
. This makes them easy to reach from the .wxs file via the ROOTDIR and thensrc\app\helloworld\resources
. This differs from the repository layout where they are atsrc/helloworld/resources
, so without theapp
in between. For Windows, I think this is now solved cleanly. However, on macOS, I believe the icon paths were relative to the repository root. I may want to propose to change that when I'm working on that PR.What is left to do: if we want to change the icon path, we should document that.
PR Checklist: