[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 121447: Return inode/directory when isDir returns true (kfileitem)
From: Àlex_Fiestas <afiestas () kde ! org>
Date: 2015-02-15 18:28:39
Message-ID: 20150215182839.4989.48456 () probe ! kde ! org
[Download RAW message or body]
--===============7340111275304032407==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Feb. 9, 2015, 7:24 p.m., David Faure wrote:
> > This should not be done if the slave has provided a UDS_MIME_TYPE. E.g. kio_smb sends mimetypes that \
> > derive from inode/directory, such as application/x-smb-server and application/x-smb-workgroup. If you \
> > think this works with the current patch, please prove it with a unittest :-)
>
> Àlex Fiestas wrote:
> I quite not get this message sorry :/ can you explain a bit more?
>
> Àlex Fiestas wrote:
> Mmm I think I get it now, will check it out (write the test) tomorrow.
Added tests for the use case you suggest (I think) they pass without modification to the patch.
The reason why they patch is that UDS_MIME_TYPE is used in the ctor of the private class, so the added \
code will not we called in that case since mimetype is already initialized.
- Àlex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121447/#review75730
-----------------------------------------------------------
On Feb. 15, 2015, 6:27 p.m., Àlex Fiestas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121447/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2015, 6:27 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kio
>
>
> Description
> -------
>
> If we know that the item is a dir, return directly the correct mimetype for directories.
>
> More info of why this is needed at:
> https://git.reviewboard.kde.org/r/120909/
>
>
> Diffs
> -----
>
> autotests/kfileitemtest.h 0ee7204
> autotests/kfileitemtest.cpp 59c104e
> src/core/kfileitem.cpp 5894226
>
> Diff: https://git.reviewboard.kde.org/r/121447/diff/
>
>
> Testing
> -------
>
> Besides tests, tried smb kioslave and it worked great.
>
>
> Thanks,
>
> Àlex Fiestas
>
>
--===============7340111275304032407==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/121447/">https://git.reviewboard.kde.org/r/121447/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 9th, 2015, 7:24 p.m. UTC, <b>David Faure</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">This should not be done if the slave has provided a UDS_MIME_TYPE. E.g. \
kio_smb sends mimetypes that derive from inode/directory, such as application/x-smb-server and \
application/x-smb-workgroup. If you think this works with the current patch, please prove it with a \
unittest :-)</p></pre> </blockquote>
<p>On February 9th, 2015, 10:30 p.m. UTC, <b>Àlex Fiestas</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I quite not get this message sorry :/ can you explain a bit \
more?</p></pre> </blockquote>
<p>On February 9th, 2015, 11:04 p.m. UTC, <b>Àlex Fiestas</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Mmm I think I get it now, will check it out (write the test) \
tomorrow.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Added tests for the use case you suggest (I think) they pass without \
modification to the patch.</p> <p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The reason why they patch is that UDS_MIME_TYPE is used in the ctor of the \
private class, so the added code will not we called in that case since mimetype is already \
initialized.</p></pre> <br />
<p>- Àlex</p>
<br />
<p>On February 15th, 2015, 6:27 p.m. UTC, Àlex Fiestas wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Àlex Fiestas.</div>
<p style="color: grey;"><i>Updated Feb. 15, 2015, 6:27 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kio
</div>
<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;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">If we know that the item is a dir, return \
directly the correct mimetype for directories.</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">More info of why this is needed at: \
https://git.reviewboard.kde.org/r/120909/</p></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;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Besides tests, tried smb kioslave and it \
worked great.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>autotests/kfileitemtest.h <span style="color: grey">(0ee7204)</span></li>
<li>autotests/kfileitemtest.cpp <span style="color: grey">(59c104e)</span></li>
<li>src/core/kfileitem.cpp <span style="color: grey">(5894226)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121447/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============7340111275304032407==--
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic