• Addressing code review comments
• Adding back macro
• Allow window activation without forceActiveWindow()
• Avoiding duplication of tabs
• Distributing URLs among all instances
• Making ShowFolders/ShowItems also check if URLs are open in other tabs
• Ensuring current instance is considered when needed
• Removing unnecessary include
• Overloading openFiles/openDirectories
• Removing unnecessary new line
• Amend test
• Fixing header file

In D16648#468028, @fvogt wrote:

In D16648#467893, @feverfew wrote:

In D16648#467854, @fvogt wrote:

It looks like several QUrls were changed to QString here, why? Most of those QStrings, if not all, actually contain URLs.

If I remember correctly Qt methods that are publicised on DBus cannot have parameters as QList<QUrl>, hence I had to use QStringList. This seems to happen often enough that Qt even has helper functions to do the conversion. There are some more conversions to avoid unnecessarily changing method signatures of existing functions.

Not quite sure whether Qt supports that, but you could overload the methods of MainWindow so that the slots accept a QStringList and just pass it on.

In D16648#467854, @fvogt wrote:

It looks like several QUrls were changed to QString here, why? Most of those QStrings, if not all, actually contain URLs.

• Removing unnecessary include

• Allow window activation without forceActiveWindow()
• Avoiding duplication of tabs
• Distributing URLs among all instances
• Making ShowFolders/ShowItems also check if URLs are open in other tabs
• Ensuring current instance is considered when needed

In D16648#450590, @davidedmundson wrote:

activateWindow works if and only if your X startup info is correct.

When you launch dolphin through a UI (on gnome or Plasma) you will see an extra environment variable on your dolphin process.

DESKTOP_STARTUP_ID=David-Desktop;1555336591;362121;16299_TIME219914845

Qt normally handles this internally.

If you then call into an existing dolphin you need to pass this and import it the other side before calling window->activate().
See KStartupInfo or QX11Extras::setAppTime

Plasma isn't really involved.

In D16648#449696, @elvisangelaccio wrote:

This made me think: what about non-plasma systems?
e.g. GNOME doesn't even have a taskbar, what's gonna happen there?

• Adding back macro

• Addressing code review comments

In T10263#180285, @fvogt wrote:

In T10263#180221, @feverfew wrote:

In T10263#178973, @fvogt wrote:

@feverfew Sure! I don't know much about GSoC though, I hope that's not an issue.

@fvogt Great, I can tell you more about it on a platform of your choice. What's your preferred method of contact (.e.g email) so we can get a project submitted (deadline is 9th April)?

E-Mail is fine - you can use the one here on phab.

In D16648#404141, @elvisangelaccio wrote:

Also, can you please update the Summary and the Test Plan? We should remove mentions of the option (which we dropped) and we should use FEATURE: instead of BUG:.

• Merge branch 'master' into arcpatch-D16648

In T10263#178973, @fvogt wrote:

@feverfew Sure! I don't know much about GSoC though, I hope that's not an issue.

@fvogt I have some free time this summer and would like to help out in getting https://gitlab.com/Vogtinator/kio-fuse finished. Would you be willing to get in contact and flesh out a project proposal for GSoC 2019?

In D16648#424157, @ngraham wrote:

In D16648#424130, @feverfew wrote:

However after merging the KF5 min version has changed to 5.56 and so I am unfortunately am unable to test the code atm and don't have the time to switch disros. On that note, I currently use Kubuntu 18.10, any recommendations on a distro which doesn't have archaic versions of KDE frameworks/applications. I was thinking OpenSUSE TumbleWeed or KDE Neon, thoughts?

Easy-but-might-not-work option: change the minimum frameworks version in CMakeLists.txt to match the latest version that you have on disk. If the version was bumped just because of new icons or something, this will work. If Dolphin then fails to build because the dependency was bumped because of using new API in KIO or something, then you'll probably to use the...

Harder-but-more-correct option: build all the frameworks from source. It's not actually hard at all. Here's how: https://community.kde.org/Get_Involved/development#One-time_setup:_your_development_environment. I'm generally available to help on the #kde-devel IRC/Matrix channel. If you don't want to do this, then there's always the...

Easy-but-uses-a-ton-of-disk-space option: Put KDE Neon dev unstable in a VM and develop in that. When you compile, install to /usr. Boom, done. This does tend to take up quite a lot of space, though.

@ngraham I'll be having some free time this summer. I was thinking if we could solidify this task into exactly what we need to do so we can get a proposal in for GSoC?

In D16648#404125, @elvisangelaccio wrote:

I'm not sure if I'm testing this patch the wrong way, but it doesn't seem to work here. What I did:

1. Start patched dolphin (make sure no other instances are running)
2. Call the ShowFolders method from QDBusViewer
   
   The folder is opened in a new window, not in a new tab in the already running dolphin process.

• Fixed ShowFolder method
• Merge remote-tracking branch 'origin' into DBusTabInstance
• Refactoring test case

In D11382#417563, @ngraham wrote:

I'll take over from here.

We have some WIP work to add a "single window mode" to Dolphin where all new window calls generate new tabs in the current window instead (D16648, D19150). What do folks think about making this an option only when that mode is being used? Or would that be too restrictive?

In D16648#415799, @ngraham wrote:

@feverfew, are you still around to work on these issues?

Just a little bump, hope a review comes soon :)

Any progress on the state of this patch?

• Added braces to one line conditions

• Make new window option clearer, remove unnecessary string and whitespace

• Removed opening new tabs as an option.

In D16648#353559, @anthonyfieroni wrote:

--new-window will be more readable, about me. Add braces on one line conditions, tryRise should be raise ('try' does not make sense).

• Make new window option clearer, remove unnecessary string and whitespace

• Removed unnecessary string

@ngraham a.saoutkin@gmail.com Thanks!

@ngraham Just checked on Firefox and if you right click the last tab it allows you to choose "Close Tab" even though that will also close the window. Hence, seeming as most people aren't bothered by this behaviour in Firefox, I think we can probably just land this as is?

In D15278#340898, @elvisangelaccio wrote:
I still think this should be addressed. Maybe we can change the text of the action from "Close Tab" to "Close Window" if there is only one tab ?

I think including the patch as is would only annoy the people who hold the close tab shortcut in attempt to close all tabs bar one, whoever that may be. We could also always show the tab bar, to make it more obvious that Dolphin uses tabs (as a Windows user I always used multiple windows). I think the best option would be to add closing Dolphin via closing the last tab as a setting. However, I originally tried to do this, the problem is that when I tried to add it as a setting it would only apply when the last tab was changed (DolphinMainWidget::tabCountChanged) and the change would never show on the menubar until Dolphin was restarted. The best solution for this patch is a setting that actually works, otherwise, if the features is not deemed too unobtrusive, this should go through, even if I may be a bit biased (seeming as it is my first patch for KDE) :)

Removed line due to it being redundant