Dolphin Service Menu Installer: Launch certain scripts in Konsole
ClosedPublic

Authored by alex on Apr 22 2020, 11:59 AM.

Details

Summary

In general the issue with installers/uninstallers for the dolphin plugin is
that a lot of the scripts are intended to be run manually in a terminal.
For instance if a script uses sudo make install the user can't type in the password.

With this patch scripts that are executed without arg variants are
executed in konsole (if available).

Test Plan

Tests still pass. Try to install the "Jetbrains Dolphin Plugin" tar.gz file from the kde store.
If you have the required dependencies etc. the konsole window should close.
If the install script fails (type in wrong admin password a couple of times) you
should have a shell opened.

Diff Detail

Repository
R318 Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
alex created this revision.Apr 22 2020, 11:59 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptApr 22 2020, 11:59 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
alex requested review of this revision.Apr 22 2020, 11:59 AM
alex updated this revision to Diff 80872.Apr 22 2020, 12:00 PM

Update patch

alex updated this revision to Diff 80873.Apr 22 2020, 12:03 PM

Very cool idea, However this seems like a new feature to me such that it would need to be landed on master, not the stable branch.

alex added a comment.EditedApr 22 2020, 4:52 PM

Well, its a feature to fix existing issues ;-)
But then it will go on master.

meven added inline comments.Apr 24 2020, 6:36 AM
src/settings/services/servicemenuinstaller/servicemenuinstaller.cpp
58

s/if(!/if (!/

alex updated this revision to Diff 81067.Apr 24 2020, 6:42 AM

Simplify if statement

Is this what you meant?

src/settings/services/servicemenuinstaller/servicemenuinstaller.cpp
56–65

Given that this function is very simple and it's only called in one place, I'd just put the logic in runScriptOnce().

135

enums should be preferred to booleans as arguments in API.

alex updated this revision to Diff 81299.Apr 27 2020, 7:01 AM

Set process parameters inline, use enum

alex marked 2 inline comments as done.Apr 27 2020, 7:34 AM
This comment was removed by alex.
alex updated this revision to Diff 81751.May 2 2020, 6:11 PM
alex edited the summary of this revision. (Show Details)
alex edited the test plan for this revision. (Show Details)

Launch $SHELL in Konsole after installer fails

alex updated this revision to Diff 81752.May 2 2020, 6:13 PM

Drop KCoreAddons prefix from include

elvisangelaccio accepted this revision.May 3 2020, 5:51 PM
This revision is now accepted and ready to land.May 3 2020, 5:51 PM
This revision was automatically updated to reflect the committed changes.