Ubuntu Application Launcher

Merge lp://staging/~larryprice/ubuntu-app-launch/some-icon-paths-need-merged into lp://staging/ubuntu-app-launch/16.10

  1. some-icon-paths-need-merged
  2. Merge into trunk.16.10
Proposed by Larry Price
Status: Rejected
Rejected by: Ted Gould
Proposed branch: lp://staging/~larryprice/ubuntu-app-launch/some-icon-paths-need-merged
Merge into: lp://staging/ubuntu-app-launch/16.10
Diff against target: 104 lines (+75/-1)
2 files modified
libubuntu-app-launch/application-icon-finder.cpp (+59/-1)
tests/application-icon-finder.cpp (+16/-0)
To merge this branch: bzr merge lp://staging/~larryprice/ubuntu-app-launch/some-icon-paths-need-merged
Reviewer Review Type Date Requested Status
Ted Gould (community) Needs Fixing
unity-api-1-bot continuous-integration Needs Fixing
Review via email: mp+298942@code.staging.launchpad.net

Commit message

Some explicit icon paths need to be merged with the base path to find existing icon.

Description of the change

Some explicit icon paths need to be merged with the base path to find existing icon.

In particular, installing supertuxkart in libertine on a device with vivid+overlay will result in an icon path of "/usr/share/pixmaps/supertuxkart_128.png". IconFinder could not find this path because it did not exist on the system `/usr/share/pixmaps/`. The base search path for libertine apps includes `/usr/share` already, so a simple concatenation doesn't work. I've opted to iterate through the base path, figure out where the base path and icon name match, and merge the two together if a file exists at that location.

To post a comment you must log in.
Revision history for this message
unity-api-1-bot (unity-api-1-bot) wrote :

FAILED: Continuous integration, rev:233
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/9/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/228/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/236
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=vivid+overlay/177
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=xenial+overlay/177
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-1-sourcepkg/release=yakkety/177
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/106
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/106/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/106
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/106/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/106
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/106/artifact/output/*zip*/output.zip
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/106/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/106/console
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/106
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/106/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/106
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/106/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/106
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/106/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/106
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/106/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-ubuntu-app-launch-ci/9/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Ted Gould (ted) wrote :

We've discussed this a bit on IRC and it seems a bit more complex than we originally thought. We don't want to match all the way down the path as we don't want an application to be able reference an icon outside of the Libertine container, or the Click package, etc. We do want it to be able do a full reference inside that container.

So to do this it is likely that we'll have to create another string to pass into the Desktop() object that is the contained path for references. This seems like it could have other uses in the future as well. That contained path can then be passed to the icon finder so that it doesn't over extend its search.

review: Needs Fixing

Unmerged revisions

233. By Larry Price

Reverse the merge lookup to be more performant

232. By Larry Price

Take care of more cases

231. By Larry Price

Attempt to prepend basePath to explicit icon names

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches