Differential D21478

Fix passing local file paths on the command line
ClosedPublic
Actions

Authored by wbauer on May 29 2019, 12:47 PM.

Details

Reviewers
fvogt
Group Reviewers
Amarok
Commits
R181:cea336303aaf: Fix passing local file paths on the command line
Summary

Use QUrl::fromUserInput() to handle local file paths (without "file://") as well.
QUrl::AssumeLocalFile is passed to avoid DNS lookups for non-existent/mistyped file names.

Test Plan

Created a file 1d5d9dD.oga in /home/wolfi/Desktop/ (anything will do of course).
Ran:

  • amarok file:///home/wolfi/Desktop/1d5d9dD.oga
  • amarok /home/wolfi/Desktop/1d5d9dD.oga
  • cd /home/wolfi/Desktop ; amarok 1d5d9dD.oga

The file was opened and played sucessfully in all cases, whereas before only the first one worked (with phonon-backend-gstreamer at least).

Tested with both phonon-backend-gstreamer and phonon-backend-vlc.

Diff Detail

Repository
R181 Amarok
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
wbauer created this revision.May 29 2019, 12:47 PM
Restricted Application removed a project: Amarok. · View Herald TranscriptMay 29 2019, 12:47 PM
Restricted Application added a subscriber: amarok-devel. · View Herald Transcript
wbauer requested review of this revision.May 29 2019, 12:47 PM
wbauer edited the summary of this revision. (Show Details)
wbauer edited the summary of this revision. (Show Details)
wbauer edited the test plan for this revision. (Show Details)
fvogt added a subscriber: fvogt.May 29 2019, 1:01 PM
fvogt added inline comments.
src/App.cpp
280

Did it previously also handle relative paths such as ./foo.ogg? If so, passing QDir::currentPath() instead of a null string is AFAICT necessary.

wbauer added inline comments.May 29 2019, 1:14 PM
src/App.cpp
280

It does handle relative paths just fine, see the test plan. (I just tried ./foo.ogg to be sure)
QUrl::fromUserInput() passes the directory to QDir(), which assumes '.' if empty.

I just noticed that amarok has a "--cwd" command line option though (that is currently ignored AFAICS), it would probably be good to pass that instead.

fvogt accepted this revision.May 29 2019, 1:18 PM
This revision is now accepted and ready to land.May 29 2019, 1:18 PM
wbauer marked an inline comment as done.May 29 2019, 3:14 PM
wbauer added inline comments.
src/App.cpp
280

I just noticed that amarok has a "--cwd" command line option though (that is currently ignored AFAICS), it would probably be good to pass that instead.

Hm, the "--cwd" option is not even parsed correctly (the QCommandLineOption doesn't specify that it actually takes an argument, therefore it's always empty if set).

So I'll leave this patch as-is.

Closed by commit R181:cea336303aaf: Fix passing local file paths on the command line (authored by wbauer). · Explain WhyJun 14 2019, 12:03 PM
This revision was automatically updated to reflect the committed changes.
wbauer marked an inline comment as done.

Revision Contents
Changeset List

PathPackages
Msrc/App.cpp (2 lines)

Diff 59786

View Options

src/App.cpp

Loading...