<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/5186/">http://svn.reviewboard.kde.org/r/5186/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 29th, 2010, 4:50 p.m., <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this patch is fundamentally flawed - its function is to simply break the logic for home dir expansion entirely.
the correct approach is relying on tildeExpand()'s documented escaping behavior. i think it would make most sense to first run the string through tildeExpand unconditionally and only then check whether the path is absolute. and document the escaping in some user-visible places.</pre>
</blockquote>
<p>On August 29th, 2010, 7:34 p.m., <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">addendum: my idea won't work, as kurl::setPath() calls tildeExpand() itself. while that is a nice convenience feature, such functions at such a low level pretty much always backfire, like in this case ...
to fix this, we need to add an enum parameter to setPath() which would tell it not to call tildeExpand().</pre>
</blockquote>
<p>On September 8th, 2010, 4:54 p.m., <b>Dawit Alemayehu</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Actually to me this seems to be caused by the logic used in KShell::tildeExpand. It is correct that KUrl::setPath has no business silently expanding the tilde character ; specially since the use of it in the path component of a url is prefectly valid according to RFC 2396. However, KShell::tildeExpand returns an empty string when you pass it "~blah", where "blah" is not a valid user, instead of returning "~blah" back. Unless that is intentional, it is the source of this bug.</pre>
</blockquote>
<p>On September 8th, 2010, 5:52 p.m., <b>Oswald Buddenhagen</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this is not the source of the bug, but it just exposes it.
the tildeExpand() behavior is kind of intentional - the bash-compliant behavior of not touching unresolvable users introduces an ambiguity, however, it is obviously more convenient.
</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">So, what can i do with this? I really have no idea which code part is the one that is bugged here besides the proposed patches...
Perhaps you guys with more in depth knowledge about this subject can figure out how it should work and patch accordingly?</pre>
<br />
<p>- Mark</p>
<br />
<p>On August 29th, 2010, 7:45 p.m., Mark wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Mark.</div>
<p style="color: grey;"><i>Updated 2010-08-29 19:45:14</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch fixes https://bugs.kde.org/show_bug.cgi?id=117473 and allows "~" to be the folder name.
this is also consistent with new file names that could already have the "~" name.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested the patch and seems to work fine.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=117473">117473</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/KDE/kdelibs/kfile/knewfilemenu.cpp <span style="color: grey">(1169591)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5186/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>