<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/123806/">https://git.reviewboard.kde.org/r/123806/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 16th, 2015, 9:37 p.m. UTC, <b>Patrick Eigensatz</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/123806/diff/5/?file=369605#file369605line91" style="color: black; font-weight: bold; text-decoration: underline;">klipper/historyitem.cpp</a>
<span style="font-weight: normal;">
(Diff revision 5)
</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; ">HistoryItemPtr HistoryItem::create( QDataStream& dataStream ) {</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">91</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">Klipper</span><span class="p">.</span><span class="n">m_bTrimBlankEntries</span> <span class="o">&&</span> <span class="n">text</span><span class="p">.</span><span class="n">isEmpty</span><span class="p">()</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm not sure if I can access "Klipper" from here. If I have a look at how this is done at other options then I see that they only use the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">m_bSomeSetting</em> vars inside the klipper class itself.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I need some help on how I should access the setting. (Pass it? Check it in the klipper class?)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thank you :)</p></pre>
</blockquote>
<p>On May 17th, 2015, 10:28 a.m. UTC, <b>Thomas Lübking</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;">ensure klippersettings.h is included (ie. add it, implicit inclusion is a timebomb) and use KlipperSettings::trimBlankEntries()</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"Klipper" is a class and you cannot access a class this way.
If m_bTrimBlankEntries <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">was</em> (don't make it!) a static public member, you could access it via Klipper::m_bTrimBlankEntries (after including klipper.h here)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You're btw. expected to compile the changes.
Een reviews will easily oversee missing semicolons etc. - compilers do a far better job itr. ;-)</p></pre>
</blockquote>
<p>On May 17th, 2015, 10:52 a.m. UTC, <b>Patrick Eigensatz</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;">Ah, I thougth we would load the settings from KlipperSettings:: into Klipper - an instance of the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">klipper</em> class - and then never use it again. If I can simply include klippersettings.h and get the value
from there, that's fine. Thank you!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yes, I'm very sorry about this. I've spent hours and nights configuring kdesrc-build and I've tried to compile klipper in a Kubuntu-VM, followed the whole Plasma/Building guide - still didn't work.
Now trying with cmake ;)</p></pre>
</blockquote>
<p>On May 17th, 2015, 7:12 p.m. UTC, <b>Patrick Eigensatz</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;">Just a small resumé:
1) Create an option "NAME" to simplify the strings using QString::simplify()
2) This option is enabled by default</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We only need to find a name for this option.</p></pre>
</blockquote>
<p>On May 17th, 2015, 7:29 p.m. UTC, <b>Christoph Feck</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 wouldn't enable this option by default. The default should be pasting what was copied.
Also, please do not mix options that affect the actual clipboard contents and the representation in the menu. An option that says "Replace whitespace with symbols" is highly confusing next to an option that says "Trim whitespace".</pre>
</blockquote>
<p>On May 17th, 2015, 7:36 p.m. UTC, <b>Kai Uwe Broulik</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;">Btw I just pushed the highlight feature to the Klipper plasmoid and I don't see why that one should be configurable.</p></pre>
</blockquote>
<p>On May 17th, 2015, 10:44 p.m. UTC, <b>Thomas Pfeiffer</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I wouldn't enable this option by default. The default should be pasting what was copied.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Please explain what ordinary users would need extra whitespace at the beginning or end of a selection for.</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also, please do not mix options that affect the actual clipboard contents and the representation in the menu. An option that says "Replace whitespace with symbols" is highly confusing next to an option that says "Trim whitespace".
Btw I just pushed the highlight feature to the Klipper plasmoid and I don't see why that one should be configurable.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Makes sense. Just one option for trimming, then. If it's not trimmed, it's replaced. I agree that there's no reason why one would not want the whitespace visualized in a better way.</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;">> Please explain what ordinary users would need extra whitespace at the beginning or end of a selection for.
When reordering words in a text-editor, I cut/paste them including a space, otherwise I end up with wrong (double or missing) spaces.</pre>
<br />
<p>- Christoph</p>
<br />
<p>On May 16th, 2015, 9:31 p.m. UTC, Patrick Eigensatz 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-workspace, KDE Usability and Patrick Eigensatz.</div>
<div>By Patrick Eigensatz.</div>
<p style="color: grey;"><i>Updated May 16, 2015, 9:31 p.m.</i></p>
<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=159267">159267</a>,
<a href="https://bugs.kde.org/show_bug.cgi?id=192922">192922</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</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;">[PATCH] plasma-workspace: klipper: Fix #192922 Ignore blank entries</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QString::isEmpty() is used to check if the string only consists of whitespace characters. If it does, the creation of the HistoryStringItem fails.</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>klipper/generalconfig.ui <span style="color: grey">(f513e9c)</span></li>
<li>klipper/historyitem.cpp <span style="color: grey">(36cbe61)</span></li>
<li>klipper/klipper.h <span style="color: grey">(6952b11)</span></li>
<li>klipper/klipper.cpp <span style="color: grey">(798b49f)</span></li>
<li>klipper/klipper.kcfg <span style="color: grey">(a03dd16)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123806/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>