<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/107296/">http://git.reviewboard.kde.org/r/107296/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">A few issues left, then I'd be fine with merging this as it's already a big improvement over what we have so far - thanks!
But in the long term this should definitely be improved, and I hope you will continue to do the work:
What I see here, is that we essentially redo a tree, something that should be handled by QTreeView and a proper model. To get some fancy rendering in there, we can use a custom delegate. The big advantage would be:
- we can add a filter proxy model (for search)
- it reuses existing code, and thus also integrates better into the rest of our UI, esp. for dark color scheme users
If for one reason or the other, the above is not feasible, then at least we should refactor the code to be a "ShowUsesContext" and simply return some html there. This way we can reuse the code there for embedding links etc.</pre>
<br />
<div>
<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/107296/diff/4/?file=94779#file94779line68" style="color: black; font-weight: bold; text-decoration: underline;">language/duchain/navigation/useswidget.h</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace KDevelop {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">64</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">class</span> <span class="n">KDEVPLATFORMLANGUAGE_EXPORT</span> <span class="n">NavigatableWidgetList</span> <span class="o">:</span> <span class="n">public</span> <span class="n">QScrollArea</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">68</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">class</span> <span class="n">KDEVPLATFORMLANGUAGE_EXPORT</span> <span class="n">NavigatableWidgetList</span> <span class="o">:</span> <span class="n">public</span> <span class="n">QScrollArea</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">after you've landed this patch, please feel free to move these classes into separate files as well.</pre>
</div>
<br />
<div>
<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/107296/diff/4/?file=94780#file94780line130" style="color: black; font-weight: bold; text-decoration: underline;">language/duchain/navigation/useswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QString highlightAndEscapeUseText(QString line, uint cutOff, SimpleRange range) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">120</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">text</span> <span class="o">=</span><span class="hl"> </span><span class="n"><span class="hl">sizePrefix</span></span><span class="hl"> </span><span class="o"><span class="hl">+</span></span> <span class="s">"<a href='open'>"</span> <span class="o">+</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Line"</span><span class="p">)</span> <span class="o">+</span> <span class="n"><span class="hl">sizeSuffix</span></span><span class="hl"> </span><span class="o"><span class="hl">+</span></span><span class="hl"> </span><span class="n">QString</span><span class="p">(</span><span class="s">" <b>%1<span class="hl">%2%3</span></b></a>"</span><span class="p"><span class="hl">).</span></span><span class="n"><span class="hl">arg</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">sizePrefix</span></span><span class="p">).</span><span class="n">arg</span><span class="p">(</span><span class="n">range</span><span class="p">.</span><span class="n">start</span><span class="p">.</span><span class="n">line</span><span class="p">)<span class="hl">.</span></span><span class="n"><span class="hl">arg</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">sizeSuffix</span></span><span class="p"><span class="hl">)</span>;</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">128</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">text</span> <span class="o">=</span> <span class="s">"<a href='open'>"</span> <span class="o">+</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Line"</span><span class="p">)</span> <span class="o">+</span> <span class="n">QString</span><span class="p">(</span><span class="s">" <b>%1</b><span class="hl">:</span></a>"</span><span class="p">).</span><span class="n">arg</span><span class="p">(</span><span class="n">range</span><span class="p">.</span><span class="n">start</span><span class="p">.</span><span class="n">line</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">i18n puzzle</pre>
</div>
<br />
<div>
<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/107296/diff/4/?file=94780#file94780line180" style="color: black; font-weight: bold; text-decoration: underline;">language/duchain/navigation/useswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void OneUseWidget::resizeEvent ( QResizeEvent * event ) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">160</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setText</span><span class="p">(</span><span class="n">i18n</span><span class="p">(</span><span class="s">"<a href='open'>Line <b>%1</b></a> %2"</span><span class="p">,</span> <span class="n">range</span><span class="p">.</span><span class="n">start</span><span class="p">.</span><span class="n">line</span><span class="o">+</span><span class="mi">1</span><span class="p">,</span> <span class="n">highlightAndEscapeUseText</span><span class="p">(</span><span class="n">m_sourceLine</span><span class="p">,</span> <span class="n">cutOff</span><span class="p">,</span> <span class="n">range</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">177</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"> </span><span class="n"><span class="hl">m_label</span></span><span class="o"><span class="hl">-></span></span><span class="n">setText</span><span class="p">(</span><span class="n">i18n</span><span class="p">(</span><span class="s">"<a href='open'>Line <b>%1</b></a> %2"</span><span class="p">,</span> <span class="n">range</span><span class="p">.</span><span class="n">start</span><span class="p">.</span><span class="n">line</span><span class="o">+</span><span class="mi">1</span><span class="p">,</span> <span class="n">highlightAndEscapeUseText</span><span class="p">(</span><span class="n">m_sourceLine</span><span class="p">,</span> <span class="n">cutOff</span><span class="p">,</span> <span class="n">range</span><span class="p">)));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">i18n context missing</pre>
</div>
<br />
<div>
<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/107296/diff/4/?file=94780#file94780line184" style="color: black; font-weight: bold; text-decoration: underline;">language/duchain/navigation/useswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void OneUseWidget::resizeEvent ( QResizeEvent * event ) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">164</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setText</span><span class="p">(</span><span class="n">i18n</span><span class="p">(</span><span class="s">"<a href='open'>Line <b>%1</b></a> %2"</span><span class="p">,</span> <span class="n">range</span><span class="p">.</span><span class="n">start</span><span class="p">.</span><span class="n">line</span><span class="o">+</span><span class="mi">1</span><span class="p">,</span> <span class="n">highlightAndEscapeUseText</span><span class="p">(</span><span class="n">m_sourceLine</span><span class="p">,</span> <span class="n">cutOff</span><span class="p">,</span> <span class="n">range</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">181</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">m_label</span></span><span class="o"><span class="hl">-></span></span><span class="n">setText</span><span class="p">(</span><span class="n">i18n</span><span class="p">(</span><span class="s">"<a href='open'>Line <b>%1</b></a> %2"</span><span class="p">,</span> <span class="n">range</span><span class="p">.</span><span class="n">start</span><span class="p">.</span><span class="n">line</span><span class="o">+</span><span class="mi">1</span><span class="p">,</span> <span class="n">highlightAndEscapeUseText</span><span class="p">(</span><span class="n">m_sourceLine</span><span class="p">,</span> <span class="n">cutOff</span><span class="p">,</span> <span class="n">range</span><span class="p">)));</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">also context missing</pre>
</div>
<br />
<div>
<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/107296/diff/4/?file=94780#file94780line453" style="color: black; font-weight: bold; text-decoration: underline;">language/duchain/navigation/useswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">ContextUsesWidget::ContextUsesWidget(const CodeRepresentation& code, QList<IndexedDeclaration> usedDeclarations, IndexedDUContext context) : m_context(context) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">417</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">label</span><span class="o">-></span><span class="n">setText</span><span class="p">(</span><span class="n">i18np</span><span class="p">(</span><span class="s">"<b>File:</b> %2 <i>(1 use)</i>"</span><span class="p">,</span> <span class="s">"<b>File:</b> %2 <i>(%1 uses)</i>"</span><span class="p">,</span> <span class="n">usesCount</span><span class="p">,</span> <span class="n">ICore</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span class="o">-></span><span class="n">projectController</span><span class="p">()</span><span class="o">-></span><span class="n">prettyFileName</span><span class="p">(</span><span class="n">topContext</span><span class="p">.</span><span class="n">url</span><span class="p">().</span><span class="n">toUrl</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">444</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">labelText</span> <span class="o">=</span> <span class="n">i18np</span><span class="p">(</span><span class="s">"<b>1 use:</b> %2"</span><span class="p">,</span> <span class="s">"<b>%1 uses:</b> %2"</span><span class="p">,</span><span class="ew"> </span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">context for %2 at least missing</pre>
</div>
<br />
<div>
<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/107296/diff/4/?file=94780#file94780line648" style="color: black; font-weight: bold; text-decoration: underline;">language/duchain/navigation/useswidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void UsesWidget::UsesWidgetCollector::maximumProgress(uint max) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">551</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">expand</span> <span class="o">=</span> <span class="kc">true</span><span class="p">;</span> <span class="c1">//Expand + move to front the item belonging to the current open document</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">635</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">bool</span> <span class="n">toFront</span> <span class="o">=</span> <span class="n">ICore</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span class="o">-></span><span class="n">documentController</span><span class="p">()</span><span class="o">-></span><span class="n">activeDocument</span><span class="p">()</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">put the active doc into a temp var and reuse that to reduce the line length</pre>
</div>
<br />
<p>- Milian</p>
<br />
<p>On November 14th, 2012, 9:19 p.m., Sven Brauch 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, KDE Usability and Milian Wolff.</div>
<div>By Sven Brauch.</div>
<p style="color: grey;"><i>Updated Nov. 14, 2012, 9:19 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;">TL;DR: Look at the screenshots below. :)
Make the uses widget nicer.
Main changes include:
* Fix contexts not being clickable (they were clickable but didn't work)
* Remove the blue arrows; all widgets are expanded by default and can
be collapsed with a small link in the text instead. This operation is
not common enough to justify such a huge ugly button.
(The main problem with the button was that it looked ugly, tough)
* Contexts without actual uses are always sorted last.
* Add some nice icons to the list.
* Remove all the ugly frames.
* Make text displaying code use a fixed-width font.
* Make the tooltip use a fixed-width font too, make it smaller (just 2
lines of context instead of 3), and highlight the line with the use
by making it bold-face.
* Re-format and rename all the widget headers to be more readable
and look nicer.
I will add a few comments to the patch to explain what I was doing.
Oh and also, I had that impression for quite a while now, but right now I'm pretty sure the find uses feature is broken in some way. Just look at the screenshots: it found way more occurences in the first screenshot than in the second (which happened after a reparse of some projects). And this has nothing to do with my changes. :)</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;">Manual testing; no functional changes were done except for the context link fix.</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>language/duchain/navigation/useswidget.h <span style="color: grey">(ec0440e)</span></li>
<li>language/duchain/navigation/useswidget.cpp <span style="color: grey">(8c01e7c)</span></li>
<li>shell/projectcontroller.cpp <span style="color: grey">(6168066)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107296/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>
<div>
<a href="http://git.reviewboard.kde.org/r/107296/s/821/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/11/12/useswidgetcomp_400x100.png" style="border: 1px black solid;" alt="Comparsion between the old and the new uses widget." /></a>
<a href="http://git.reviewboard.kde.org/r/107296/s/822/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/11/12/useswidget_new4_400x100.png" style="border: 1px black solid;" alt="The new useswidget after the latest updates" /></a>
<a href="http://git.reviewboard.kde.org/r/107296/s/832/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/11/14/useswidget_400x100.png" style="border: 1px black solid;" alt="Third version; the header line updates while uses are being searched" /></a>
</div>
</td>
</tr>
</table>
</div>
</body>
</html>