Support opening new sessions in current working dir
ClosedPublic

Authored by muesli on Jan 18 2020, 2:34 AM.

Details

Summary

When a Konsole profile has the "Start in same directory as current session"
setting enabled, Yakuake now respects the setting and opens new sessions
in the current working dir, just like Konsole would.

BUG: 396472
FIXED-IN: 20.04.0

Diff Detail

Repository
R369 Yakuake
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
muesli requested review of this revision.Jan 18 2020, 2:34 AM
muesli created this revision.
muesli added a reviewer: hein.Jan 18 2020, 2:37 AM

Compiles and works for me.

It's inconsistent for me, mainly with regards to opening new panes.

Right after launching Yakuake, if I make a new tab or split Left/Right or Top/Bottom, Yakuake will open in the current working directory instead of the default home directory, as expected.

There seems to be a little bit of lag as well, though I'm not sure what can be done about that. If I change my current directory and open a new tab too quickly, it will open to the previous directory instead of my current one - pretty much if I try to spawn a new tab before the title bar can update with the new directory, this will happen. Don't think much can be done about this.

However, if I change directories and split left/right or top/bottom, Yakuake will open the new session in the original directory that it was in when I launched Yakuake. Using "two terminals, split horizontally" works fine, but "split left/right" which makes a new pane to the right of your current session always opens to the wrong directory. Not sure what's different about that process compared to opening a new tab or new set of split terminals.

In D26744#625119, @mweepigeon wrote:

Right after launching Yakuake, if I make a new tab or split Left/Right or Top/Bottom, Yakuake will open in the current working directory instead of the default home directory, as expected.

There seems to be a little bit of lag as well, though I'm not sure what can be done about that. If I change my current directory and open a new tab too quickly, it will open to the previous directory instead of my current one - pretty much if I try to spawn a new tab before the title bar can update with the new directory, this will happen. Don't think much can be done about this.

You're right, there's probably little we can do: we need to wait for the shell to update the cwd, konsole to pick up on it and the signals to be propagated through to Yakuake. However I'm not sure if this is really much of an issue. I can only forcefully trigger it within milliseconds after changing the directory. While regularly using my terminal I've never ran into this situation. It's also worth noting that Konsole itself behaves the same way.

However, if I change directories and split left/right or top/bottom, Yakuake will open the new session in the original directory that it was in when I launched Yakuake. Using "two terminals, split horizontally" works fine, but "split left/right" which makes a new pane to the right of your current session always opens to the wrong directory. Not sure what's different about that process compared to opening a new tab or new set of split terminals.

Thanks, I haven't discovered that yet and will look into it!

I'd also like to turn around the order of the Terminal constructor's parameters and keep the parent-widget as the last parameter, as it commonly is for KDE/Qt classes.

muesli updated this revision to Diff 77342.Mar 10 2020, 11:10 AM
  • Inherit CWD when opening a new Terminal in a split-view

@mweepigeon I've addressed the issues you've discovered. Newly opened split-panes now also inherit the current working directory, like they would in Konsole.

andrewfhou accepted this revision.Mar 10 2020, 1:26 PM

@mweepigeon I've addressed the issues you've discovered. Newly opened split-panes now also inherit the current working directory, like they would in Konsole.

Split-panes now working for me as well. Looks great!

This revision is now accepted and ready to land.Mar 10 2020, 1:26 PM
muesli edited the summary of this revision. (Show Details)Mar 10 2020, 3:00 PM

LGTM modulo a few nitpicks:

app/terminal.cpp
308

currentWorkingDirectory ()

308

I mean of course that this should be currentWorkingDirectory() :)

app/terminal.h
32–33

Might be nice to add a comment here explaining why we're using the V2 version

69

currentWorkingDirectory()

ngraham edited the summary of this revision. (Show Details)Mar 10 2020, 3:24 PM
muesli updated this revision to Diff 77357.Mar 10 2020, 3:32 PM
  • Remove superfluous space and add comment regarding TerminalInterfaceV2
ngraham accepted this revision.Mar 10 2020, 3:50 PM

LGTM!

This revision was automatically updated to reflect the committed changes.