> On Jan. 29, 2015, 7:46 a.m., David Faure wrote: > > Shouldn't splitArgs be fixed instead? After all a real shell would not > > split on that character, would it? > > Albert Astals Cid wrote: > That is indeed a good question. Bash doesn't seem to need the quotes, but > i don't know about other shells, and i figured out adding some extra quotes > for safety isn't really a problem and it's actually easier to fix and will > have less regressions (since i want this backported to kdelibs too). But if > you really prefer change in splitArgs I can have a look.
Hmm ok, yeah, I guess extra quoting doesn't hurt. This code exists to avoid quoting everything single argument since that would look awful, but in corner cases like this, no harm done indeed. I don't object to this patch, but I still think splitArgs has a corner case bug, if it's splitting where it shouldn't. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122300/#review74967 ----------------------------------------------------------- On Jan. 28, 2015, 11:24 p.m., Albert Astals Cid wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122300/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2015, 11:24 p.m.) > > > Review request for KDE Frameworks, David Faure, Eike Hein, and Michael Pyne. > > > Bugs: 343380 > https://bugs.kde.org/show_bug.cgi?id=343380 > > > Repository: kcoreaddons > > > Description > ------- > > We need to also quote spaces like the Unicode character 'IDEOGRAPHIC SPACE' > (U+3000) in KShell::quoteArg since KShell::splitArgs is using QChar::isSpace > to decide when a new argument starts, without the quoting, one argument will > get turned into two > > > Diffs > ----- > > autotests/kshelltest.cpp 451bbe9 > src/lib/util/kshell_unix.cpp e0e884f > > Diff: https://git.reviewboard.kde.org/r/122300/diff/ > > > Testing > ------- > > New test passes, bug 343380 is fixed > > > Thanks, > > Albert Astals Cid > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
