Changeset View
Standalone View
src/dolphintabwidget.cpp
| Show First 20 Lines • Show All 328 Lines • ▼ Show 20 Line(s) | 324 | { | |||
|---|---|---|---|---|---|
| 329 | const DolphinTabPage* tabPage = tabPageAt(index); | 329 | const DolphinTabPage* tabPage = tabPageAt(index); | ||
| 330 | args << tabPage->primaryViewContainer()->url().url(); | 330 | args << tabPage->primaryViewContainer()->url().url(); | ||
| 331 | if (tabPage->splitViewEnabled()) { | 331 | if (tabPage->splitViewEnabled()) { | ||
| 332 | args << tabPage->secondaryViewContainer()->url().url(); | 332 | args << tabPage->secondaryViewContainer()->url().url(); | ||
| 333 | args << QStringLiteral("--split"); | 333 | args << QStringLiteral("--split"); | ||
| 334 | } | 334 | } | ||
| 335 | args << QStringLiteral("--new-window"); | 335 | args << QStringLiteral("--new-window"); | ||
| 336 | 336 | | |||
| 337 | KIO::CommandLauncherJob *job = new KIO::CommandLauncherJob("dolphin", args, this); | 337 | KIO::CommandLauncherJob *job = new KIO::CommandLauncherJob(QCoreApplication::applicationFilePath(), args, this); | ||
elvisangelaccio: Why is this needed? Can't we fix `KIO::CommandLauncherJob` to also make it work on MacOS? Or at… | |||||
I don't see how we can fix the CommandLauncherJob; how is it to know that it needs to run applicationFilePath() instead of the provided command string? Or how is it to know, even on Mac, that a given command cannot be run as a POSIX executable but needs to be started through LaunchServices, or that it needs to dig out the "BundleExecutable"? Not to mention the fact that LaunchServices will launch *a* copy of a given application if you only specify the name and not a full path. That's worse than "the first hit on the $PATH will be launched". Using applicationFilePath() here works around all that and has the added benefit that it ensures on all platforms that the newly opened window really behaves the same way as the existing window (as far as it's supposed to). That seems so useful to me that I would have presented it in a dedicated PR if the change hadn't been required for this patch. As to fixing the documentation: I think that anyone developing on/for Mac (and esp. working on KDEis intimately aware of the fact that there are 2 ways to start applications, and what that implies. You'd need to fix the documentation for all APIs that can start applications. rjvbb: I don't see how we can fix the CommandLauncherJob; how is it to know that it needs to run… | |||||
| 338 | job->setDesktopName(QStringLiteral("org.kde.dolphin")); | 338 | job->setDesktopName(QStringLiteral("org.kde.dolphin")); | ||
| 339 | job->start(); | 339 | job->start(); | ||
| 340 | 340 | | |||
| 341 | closeTab(index); | 341 | closeTab(index); | ||
| 342 | } | 342 | } | ||
| 343 | 343 | | |||
| 344 | void DolphinTabWidget::openNewActivatedTab(int index) | 344 | void DolphinTabWidget::openNewActivatedTab(int index) | ||
| 345 | { | 345 | { | ||
| ▲ Show 20 Lines • Show All 102 Lines • Show Last 20 Lines | |||||
Why is this needed? Can't we fix KIO::CommandLauncherJob to also make it work on MacOS? Or at least fix its documention to recommend the usage of CoreApplication::applicationFilePath() ?