Add NSIS Script for KDE Connect
ClosedPublic

Authored by brute4s99 on Jun 5 2019, 2:00 PM.

Details

Summary

added script for NSIS.
This can be a model blueprint for any other apps trying out the new KNotifications for Windows.

Test Plan

checkout this branch of knotifications in craft : https://phabricator.kde.org/source/knotifications/history/brute4s99%252Fsnore-ftw/

[CraftRoot\etc\blueprints\locations\craft-blueprints-kde]
apply this diff and : https://phabricator.kde.org/D21588

[CraftRoot\craft]
apply this diff to craft : https://phabricator.kde.org/D21615

do craft --package kdeconnect-kde

EXPECTED : The setup created will-

  • install shortcut in Start Menu via SnoreToast,
  • create shortcut on Desktop for launching KDE Connect.

Diff Detail

Repository
R877 Craft Blueprints for KDE
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
brute4s99 requested review of this revision.Jun 5 2019, 2:00 PM
brute4s99 created this revision.

The only thing I immediately see as a problem is the uninstaller doesn't have the proper branding (generic icons), and it didn't uninstall the desktop shortcut

we don't provide an option for custome uninstall icons and using the app icon would be confusing.

maybe we should introduce un_sections to remove the shortcuts?

extragear/kdeconnect-kde/kdeconnect-kde.py
33

no need to set, default is display name

34

just leave it to the default

brute4s99 edited the test plan for this revision. (Show Details)Jun 5 2019, 7:46 PM
brute4s99 edited the test plan for this revision. (Show Details)Jun 5 2019, 10:05 PM
brute4s99 updated this revision to Diff 59235.Jun 5 2019, 10:14 PM

the new diff has correct shortcut installations and removals upon un-installation. be sure to update craft by

craft craft

before testing :)

brute4s99 updated this revision to Diff 59239.Jun 5 2019, 10:56 PM
vonreth added inline comments.Jun 6 2019, 10:21 AM
extragear/kdeconnect-kde/kdeconnect-kde.py
60

This one is wrong, see the condition above, I guess we need to update the snorensis script to use the correct context instead

vonreth added inline comments.Jun 6 2019, 10:24 AM
extragear/kdeconnect-kde/kdeconnect-kde.py
60

Wrong, snoretoast needs to allow absolute paths to install, so that the shortcut is installed correctly for all users

brute4s99 updated this revision to Diff 59272.Jun 6 2019, 5:13 PM

SnoreToast supports absolute path now!

brute4s99 updated this revision to Diff 59353.Jun 7 2019, 5:54 PM
brute4s99 edited the summary of this revision. (Show Details)

removed context-based shortcut handing

vonreth accepted this revision.Jun 7 2019, 6:33 PM
This revision is now accepted and ready to land.Jun 7 2019, 6:33 PM
sredman accepted this revision.Jun 8 2019, 2:59 AM
brute4s99 marked 4 inline comments as done.Jun 18 2019, 9:59 AM
vonreth added inline comments.Jun 18 2019, 12:02 PM
extragear/kdeconnect-kde/kdeconnect-kde.py
30–32

I think you should not set the execuatable, firstly he key is deprecated use shortcuts instead and you handle the installation of shortcuts in "sections"

brute4s99 updated this revision to Diff 60093.Jun 19 2019, 11:29 PM

removed executable key since it's deprecated ✂

brute4s99 marked an inline comment as done.Jun 19 2019, 11:29 PM

Has this been landed yet? Is there any reason to keep it open?

We still haven't landed KNotifications patch, so I'm waiting on that, since this patch uses SnoreToast call.
We could land it anyway, and instructions related to SnoreToast will just start working after landing the KNotifications patch(es)

vonreth accepted this revision.Jul 3 2019, 7:27 AM

ship it.
Snore toast will already get installed but the notifications will be missing.

okay. Landing it. \o/

This revision was automatically updated to reflect the committed changes.