<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/128290/">https://git.reviewboard.kde.org/r/128290/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 27th, 2016, 8:36 a.m. UTC, <b>Milian Wolff</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="https://git.reviewboard.kde.org/r/128290/diff/1/?file=469961#file469961line178" style="color: black; font-weight: bold; text-decoration: underline;">language/backgroundparser/backgroundparser.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<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">178</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">delay</span> <span class="o">=</span> <span class="o">-</span><span class="mi">1</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">also, looking below, what if delay is -2? could the special values leak here?</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">No; no update will be scheduled by DocumentChangeTracker when the delay is ILanguageSupport::NoUpdateRequired. I also replaced the other magic values by this enum, sorry for not doing that earlier.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 27th, 2016, 8:36 a.m. UTC, <b>Milian Wolff</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="https://git.reviewboard.kde.org/r/128290/diff/1/?file=469964#file469964line161" style="color: black; font-weight: bold; text-decoration: underline;">language/backgroundparser/documentchangetracker.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">161</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <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">languageController</span><span class="p">()</span><span class="o">-></span><span class="n">backgroundParser</span><span class="p">()</span><span class="o">-></span><span class="n">addDocument</span><span class="p">(</span><span class="n">m_url</span><span class="p">,</span><span class="hl"> </span><span class="n"><span class="hl">TopDUContext</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">AllDeclarationsContextsAndUses</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">136</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <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">languageController</span><span class="p">()</span><span class="o">-></span><span class="n">backgroundParser</span><span class="p">()</span><span class="o">-></span><span class="n">addDocument</span><span class="p">(</span><span class="n">m_url</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">:S</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Not opening an issue, but do you have any idea how to cleanup this API? Maybe move the delay forward so we can keep more methods as default? I think we may run into more such issues more often in the future... It's a pity we don't have named arguments in C++, nor C's designated initializers :-/</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We could introduce a struct DocumentParseRequest { IndexedString url; int priority; ParseJob::SequentialProcessingFlags flags; int delay_ms; }, with sensible default values. That would at least make it easier to read ... but yes, C++ not having named arguments is a realy pity :(</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 27th, 2016, 8:36 a.m. UTC, <b>Milian Wolff</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="https://git.reviewboard.kde.org/r/128290/diff/1/?file=469964#file469964line179" style="color: black; font-weight: bold; text-decoration: underline;">language/backgroundparser/documentchangetracker.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">173</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span> <span class="p">(</span><span class="n">QChar</span> <span class="nl">c</span> <span class="p">:</span> <span class="n">string</span><span class="p">)</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">154</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">auto</span> <span class="n">languages</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">languageController</span><span class="p">()</span><span class="o">-></span><span class="n">languagesForUrl</span><span class="p">(</span><span class="n">doc</span><span class="o">-></span><span class="n">url</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hm using the max is pretty arbitrary. It could just as well be the min? In both cases you don't actually need a container though. For max e.g., just use</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">auto delay = 0;
for (...) {
delay = max(delay, lang->...);
}
return delay;
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I know it shouldn't happen, but your version would also crash if no language controller would be found for the doc.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well, it seems quite unlikely that we will ever have multiple languages in a single document and they disagree on this ... and if that happens, the case the max handles correctly at least is when one language says "I don't need an update" and the other wants one.</p></pre>
<br />
<p>- Sven</p>
<br />
<p>On June 26th, 2016, 2:43 p.m. UTC, Sven Brauch 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 KDevelop and Kevin Funk.</div>
<div>By Sven Brauch.</div>
<p style="color: grey;"><i>Updated June 26, 2016, 2:43 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</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;">This replaces the whitespaceSensitivity() API which allowed languages to decide whether they are sensitive to whitespace changes, and instead introduces a function</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">virtual int suggestedReparseDelayForChange(KTextEditor::Document* doc, const KTextEditor::Range& changedRange,
const QString& changedText, bool removal) const;
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">in ILanguageSupport which can be overriden. Languages can choose that the signalled change does not require an update at all (white-space in some languages), that an update should happen with the default delay, or give a custom delay in ms. The default implementation indicates an update with the default delay if the change was not whitespace-only or was whitespace but adjacent to a word.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In kdev-clang, this enables us to do something like</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"><span style="color: #008000; font-weight: bold">int</span> <span style="color: #008000; font-weight: bold">ClangSupport</span><span style="color: #666666">:</span><span style="color: #AA22FF">:suggestedReparseDelayForChange</span><span style="color: #666666">(</span><span style="color: #008000; font-weight: bold">KTextEditor</span><span style="color: #666666">:</span><span style="color: #AA22FF">:Document</span><span style="color: #666666">*</span> <span style="color: #408080; font-style: italic">/*doc*/</span><span style="color: #666666">,</span> <span style="color: #008000; font-weight: bold">const</span> <span style="color: #008000; font-weight: bold">KTextEditor</span><span style="color: #666666">:</span><span style="color: #AA22FF">:Range</span><span style="color: #666666">&</span> <span style="color: #408080; font-style: italic">/*changedRange*/</span><span style="color: #666666">,</span>
<span style="color: #008000; font-weight: bold">const</span> <span style="color: #008000; font-weight: bold">QString</span><span style="color: #666666">&</span> <span style="color: #008000; font-weight: bold">changedText</span><span style="color: #666666">,</span> <span style="color: #008000; font-weight: bold">bool</span> <span style="color: #408080; font-style: italic">/*removal*/</span><span style="color: #666666">)</span> <span style="color: #008000; font-weight: bold">const</span>
{
if ( changedText<span style="color: #666666">.</span>contains(QLatin1Char(<span style="color: #BA2121">'\n'</span>)) <span style="color: #666666">||</span> changedText<span style="color: #666666">.</span>contains(QLatin1Char(<span style="color: #BA2121">';'</span>)) ) <span style="border: 1px solid #FF0000">{</span>
return ILanguageSupport<span style="color: #666666">::</span>DefaultDelay;
}
<span style="color: #008000; font-weight: bold">return</span> <span style="color: #008000; font-weight: bold">3000</span><span style="color: #666666">;</span>
<span style="border: 1px solid #FF0000">}</span>
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(will submit separately), which makes completion feel much more snappy (Kevin knows ;)). I actually think I will do something similar in kdev-python, namely recommend an instant re-parse when a newline is added.</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>language/backgroundparser/backgroundparser.h <span style="color: grey">(bf22d38)</span></li>
<li>language/backgroundparser/backgroundparser.cpp <span style="color: grey">(84e4011)</span></li>
<li>language/backgroundparser/documentchangetracker.h <span style="color: grey">(079feab)</span></li>
<li>language/backgroundparser/documentchangetracker.cpp <span style="color: grey">(4ddded5)</span></li>
<li>language/interfaces/ilanguagesupport.h <span style="color: grey">(5c3fc30)</span></li>
<li>language/interfaces/ilanguagesupport.cpp <span style="color: grey">(67aab36)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/128290/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>