This revision was automatically updated to reflect the committed changes.
Closed by commit R241:5f2be012c27d: [CommandLauncherJob] Add constructor taking
an executable and argument list (authored by broulik).
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28515?vs
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
Awesome.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D28515
To: broulik, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, b
broulik requested review of this revision.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D28515
To: broulik, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
broulik updated this revision to Diff 79267.
broulik added a comment.
This revision is now accepted and ready to land.
- Add test for binary in path with space
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28515?vs=79189&id=79267
REVISION DETAIL
https://pha
broulik planned changes to this revision.
broulik added a comment.
Good idea
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D28515
To: broulik, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
dfaure added a comment.
You could create a subdir of a QTemporaryDir with a fixed name like "abc
def", copy `cp` or `copy.exe` in there, and then run that with an absolute path?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D28515
To: broulik, #frameworks, dfaure
Cc: k
broulik added a comment.
Not sure how I would test running a command with a space in it.. putting a
shell script in running it through QFINDTESTDATA? Not sure how to make that
sort of stuff work on Windows
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D28515
To: broul
broulik updated this revision to Diff 79189.
broulik edited the test plan for this revision.
broulik added a comment.
- Quote executable in commandline
- Add unittest
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D28515?vs=79136&id=79189
REVISION DETAIL
ht
dfaure added a comment.
Good question. I haven't seen that done in most callers, but indeed, what if
it's in a path with a space, like often happens on Windows?
It sounds like the answer is yes, it should be quoted.
The real answer is to write a CommandLauncherJob unittest for such a case,
broulik added a comment.
Ok, makes sense. Just realized: do I need to `quoteArgs` the `executable`?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D28515
To: broulik, #frameworks, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
KProcessRunner::KProcessRunner(QString cmd) calls
KProcess::setShellCommand(cmd) which ... has two modes. Either
KShell::splitArgs()[0] finds an executable that exists (checking that there
broulik created this revision.
broulik added reviewers: Frameworks, dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.
REVISION SUMMARY
More convenient than having to construct a proper escaped commandline.
TES
12 matches
Mail list logo
