Differential D2638

Update terminal emulator default value + provide a way to set a workdir explicitly
AbandonedPublic
Actions

Authored by martinkostolny on Aug 31 2016, 7:12 PM.

Details

Reviewers
None
Group Reviewers
Krusader
Summary

Currently I can see 2 issues in Krusader ability to open a terminal emulator (setting in Konfigurator -> General -> Terminal):

  • default value "konsole --nofork" is not working anmyore since "--nofork" option was removed in version 16.08.0
  • there is no way running konsole with "--workdir <dir>" option and setting a workdir explicitly

So I propose to:

  1. change default value to: "krusader --separate" which works the same way as "--nofork" (question is, does this work for the past konsole releases?)
  2. start replacing "%d" with working directory path so one can set terminal command e.g. to this "konsole --new-tab --workdir %d"

If it turns out that "--separate" option is only in new konsole releases, we can always set the default to "konsole --workdir %d" which would also be educative for new users in a way of notifying that "%d" means workdir. What do You think?

Test Plan
  1. compile
  2. in krusaderrc remove the line starting with "Terminal=" under section [General] and start Krusader over
  3. check that under Konfigurator -> General -> Terminal is the new default value
  4. start terminal (F9) on a few active tabs and check that it is always brought to front and has the workdir set according active Krusader tab
  5. change Terminal setting to "konsole --new-tab --workdir %d"
  6. start terminal (F9) on a few active tabs and check that there is only one konsole opened with tabs set to proper directory paths

Diff Detail

Repository
R167 Krusader
Lint
Lint Skipped
Unit
Unit Tests Skipped
martinkostolny updated this revision to Diff 6396.Aug 31 2016, 7:12 PM
martinkostolny retitled this revision from to Update terminal emulator default value + provide a way to set a workdir explicitly.
martinkostolny updated this object.
martinkostolny edited the test plan for this revision. (Show Details)
martinkostolny added a reviewer: Krusader.
martinkostolny set the repository for this revision to R167 Krusader.
martinkostolny added a project: Krusader.
martinkostolny added a comment.Sep 23 2016, 9:41 PM

Konsole 15.12 (included in Kubuntu 16.04) can handle option --separate. I've tested this code on Kubuntu 16.04 as well as on Arch. So I just commited this change. Feel free to reopen and correct me.

abika added a subscriber: abika.Oct 7 2016, 4:44 PM

Makes --separate any difference? Cause the process is started with proc.startDetached().
(And --separate doesn't work for me anyway, it does not fork the process. Maybe a bug in Konsole?)

martinkostolny added a comment.Oct 7 2016, 5:15 PM

For me it works roughly as expected (Konsole 16.08.1).

  • konsole (starts new konsole tab with so far for me unpredictable but persistent workdir)
  • konsole --separate (starts always new Konsole process in proper workdir)
  • konsole --new-tab (same as "konsole")

In Kubuntu 16.04 (Konsole 15.12.3) "konsole --separate" works the same as "konsole". "konsole --new-tab" works the same as in Konsole 16.08.1.

What version of Konsole do You have?

abika added a comment.Oct 8 2016, 2:02 PM

16.08.1 here too.

In D2638#55338, @martinkostolny wrote:

For me it works roughly as expected (Konsole 16.08.1).

  • konsole (starts new konsole tab with so far for me unpredictable but persistent workdir)

But this shouldn't happen!? For me it starts always a new instance with correct workdir, as expected.

  • konsole --separate (starts always new Konsole process in proper workdir)

So what does --separate actually do?
It does not fork the process: try it from another terminal, it always blocks the parent.
And it always starts a new instance/window.

  • konsole --new-tab (same as "konsole")

Correct for me: starts a new tab with correct workdir.

Wait, I will just ask on Bugzilla...

martinkostolny added a comment.Oct 9 2016, 7:03 PM

Thanks for asking in bugzilla.

But this shouldn't happen!? For me it starts always a new instance with correct workdir, as expected.

I also believe, this behaviour is not quite expected. It is certainly different from past versions of Konsole and likely a regression.

So what does --separate actually do?
It does not fork the process: try it from another terminal, it always blocks the parent.
And it always starts a new instance/window.

I think --separate just ensures Konsole start a new process (always a new instance), not necessarily a fork.

Anyway, I still think the default value for our krusader call (konsole --separate) is good choice considering we want to ensure that Konsole always start a new window by default.

abika added a comment.Oct 9 2016, 7:09 PM

Anyway, I still think the default value for our krusader call (konsole --separate) is good choice considering we want to ensure that Konsole always start a new window by default.

sure, its fine and I don't want to be nit-picking. Close this if you want.

martinkostolny abandoned this revision.Oct 9 2016, 7:19 PM

Closing. (Automatic close by commit message didn't work this time)

Revision Contents
Changeset List

PathPackages
Mkrusader/defaults.h (2 lines)
Mkrusader/krslots.cpp (5 lines)

Diff 6396

View Options

krusader/defaults.h

Loading...
View Options

krusader/krslots.cpp

Loading...