Desktop meta: strip ${SNAP} from .desktop icon #2541
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A lot of snaps are hacking the desktop file of their projects to insert
a snap-relative path for their icon of the form
`${SNAP}/path/to/icon.png`. The current Appstream implementation does
not understand this format so this commit strips the `${SNAP}` from the
start of the path if it is present before continuing as before.
Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
cmatsuoka
requested changes
Apr 24, 2019
Desktop files might include an icon path definition with a `${SNAP}`
prefix. This commit adds a test to ensure that snapcraft correctly
handles this scenario.
Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
|
Test is added :-) |
Codecov Report
@@ Coverage Diff @@
## master #2541 +/- ##
==========================================
+ Coverage 88.96% 88.97% +<.01%
==========================================
Files 204 204
Lines 13948 13950 +2
Branches 2109 2110 +1
==========================================
+ Hits 12409 12412 +3
+ Misses 1093 1092 -1
Partials 446 446
Continue to review full report at Codecov.
|
Bad spelling: applicatins != applications. Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
cmatsuoka
approved these changes
Apr 25, 2019
|
seems the unit test I added is failing. I guess I still need to do some more work... |
sergiusens
requested changes
Apr 25, 2019
Add trailing slash to string to-be-stripped (`${SNAP}/`) in appstream
code.
Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
Return an absolute icon path when stripping `${SNAP}`.
Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
sergiusens
reviewed
May 9, 2019
Co-Authored-By: Sergio Schvezov <sergio.schvezov@canonical.com>
sergiusens
approved these changes
May 9, 2019
|
looks like tests are still failing :-( |
added 3 commits
May 9, 2019 19:21
The path was not being found previously. Tests highlighted the issue and now I've fixed the function to return the expected values. The tests correctly pass now, and haven't been changed. Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
lucyllewy
changed the title
sergiusens
reviewed
May 10, 2019
sergiusens
reviewed
May 10, 2019
- remove extra line accidentally added to `snapcraft/extractors/appstream.py`. - undelete accidentally removed assertion in `tests/unit/test_meta.py` Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
sergiusens
approved these changes
May 10, 2019
Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
clobrano
pushed a commit
to clobrano-forks/snapcraft
that referenced
this pull request
Jun 8, 2019
A lot of snaps are hacking the desktop file of their projects to insert
a snap-relative path for their icon of the form
`${SNAP}/path/to/icon.png`, ensure this is taken into account while
checking if the icon exists.
Signed-off-by: Daniel Llewellyn <daniel@bowlhat.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A lot of snaps are hacking the desktop file of their projects to insert a snap-relative path for their icon of the form
${SNAP}/path/to/icon.png. The current .desktop extractor implementation does not understand this format so this commit strips the${SNAP}from the start of the path if it is present before continuing as before.Signed-off-by: Daniel Llewellyn daniel@bowlhat.net
./runtests.sh static?./runtests.sh tests/unit?