<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://git.reviewboard.kde.org/r/107628/">http://git.reviewboard.kde.org/r/107628/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 9th, 2012, 3:30 p.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107628/diff/2/?file=97530#file97530line65" style="color: black; font-weight: bold; text-decoration: underline;">plugins/documentswitcher/documentswitcherplugin.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString truncatePath(QString path, int dirsNameLength = 18, unsigned threshold = 80)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">65</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span><span class="n">dir</span><span class="p">.</span><span class="n">length</span><span class="p">()</span> <span class="o">></span> <span class="n">maxdirsNameLength</span><span class="p">)</span> <span class="p">{</span> <span class="n">truPath</span><span class="p">.</span><span class="n">append</span><span class="p">(</span><span class="s">".."</span><span class="p">);</span> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<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 should be three dots to indicate an elide. Extra points if you could re-use Qt functionality for eliding instead of doing it manually. I'm not sure what the best way to do that would be at the moment though (maybe itemviews can do this automatically, otherwise may check what QLabel uses or so).
Also this algorithm has the downside that it elides all path parts and does not give you a guarantee that the resulting string really is smaller than 80 characts. It also may elide more than necessary, which means loss of information.</pre>
</blockquote>
<p>On December 10th, 2012, 9:08 p.m., <b>Jarosław Sierant</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;">I try to some how make paths shorter, but may be it is not necessary here (only homePath replacement would be enough). What do you think?
I want to show the user as much as it is possible.
Until width of view does not reach screen width there is no problem.
Problem is when item is longer than screen width.
There is simple solution.
When max list view size will be the same as the screen size scrallingArea take care of situation when items are longer then screen width.
Is it OK for you?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think you somewhat misunderstood what I said. I understand your intention perfectly well, shortening a long text and indicating it was shortened is usually done by 'eliding'. That means the part of the text that is removed is replaced with an ellipsis (i.e. three dots).
What I don't like about the algorithm you chose for eliding is that it considers each part of the path individually without checking wether the result is already short enough to fit. And I'm relatively sure that the KDE or Qt API already has a way of getting some text elided, at least they have an algorithm hidden somewhere since labels can already have elided text on them.
Hmm, after checking the Qt API docs, it seems like QListView should be eliding the cells text already automatically, see http://qt-project.org/doc/qt-4.8/qabstractitemview.html#textElideMode-prop. It could be of course that this depends on the style you're using.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 9th, 2012, 3:30 p.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/107628/diff/2/?file=97530#file97530line117" style="color: black; font-weight: bold; text-decoration: underline;">plugins/documentswitcher/documentswitcherplugin.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void DocumentSwitcherPlugin::walkForward()</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">87</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">DocumentSwitcherPlugin</span><span class="o">::</span><span class="n">walkForward</span><span class="p">()</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">117</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">DocumentSwitcherPlugin</span><span class="o">::</span><span class="n">setViewGeometry</span><span class="p">(</span><span class="n">Sublime</span><span class="o">::</span><span class="n">MainWindow</span><span class="o">*</span> <span class="n">window</span><span class="p">,</span> <span class="kt">int</span> <span class="n">contentWidth</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Isn't there an itemview method to resize contents to the needed size? </pre>
</blockquote>
<p>On December 10th, 2012, 9:08 p.m., <b>Jarosław Sierant</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;">I'm not sure if I understand you correctly. I want to automatically resize window to content not opposite.
I have no big experience in QT programming but I couldn't find an easy solution.
There is a problem to get correct size of the list view item.
In current implementation sizeHint is incorrect (Qt do not calculate size on its own).
So I have to calculate it manually based on the content.
I use it to resize the window.
I can reimplement sizeHint method in DocumentSwitcherTreeView class (it would be the pretty solution).
But there will be additional loop to calculate proper width.
What do you think about it?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I actually think having some maximum size significantly smaller than the screen would be better. In fact I'd say the popup shouldn't be larger than the document view to better indicate that this popup is related to the documents and not all of the IDE.
In addition I think its important to have both, the window adjust to the content and vice-versa, but that might be quite a bit more involved.
Anyway, as the code currently uses QListView there is indeed no easy way to let the view resize its column to match the contents and then resize the view to the column width. An easy way to fix that would be replacing the QListView base class with QTreeView which has a header and at least one column and thus can be told to resize the column to the contents of the column. Then you can get the column width and ensure the its not too wide. After that is done resize the column back to the width of the view in case its still wider to get the elided text from the view automatically.</pre>
<br />
<p>- Andreas</p>
<br />
<p>On December 7th, 2012, 5:25 p.m., Jarosław Sierant wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/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 KDevelop.</div>
<div>By Jarosław Sierant.</div>
<p style="color: grey;"><i>Updated Dec. 7, 2012, 5:25 p.m.</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;">Document switcher window width is automatically calculated base on content (auto adjust to longest item). Current screen width is upper bound of the window width.
Code duplication has been removed (methods walkForward/walkBackward).
A path to the file on items list can be truncated if it is too long (threshold=80).</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;">Only manual tests are done.</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>plugins/documentswitcher/documentswitcherplugin.h <span style="color: grey">(0924e81)</span></li>
<li>plugins/documentswitcher/documentswitcherplugin.cpp <span style="color: grey">(d457a5a)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107628/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>