<table><tr><td style="">davidedmundson added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D19852">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D19852#inline-112408">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kdautovic</span> wrote in <span style="color: #4b4d51; font-weight: bold;">klipper.cpp:744</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">But bIgnoreImages is used later in the code, when determining whether to save images to history or not. Is it really necessary here? If it's added here then we're back to square one because copying screenshots to clipboard won't work with the default behavior of Klipper.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">We want:</p>

<ol class="remarkup-list">
<li class="remarkup-list-item">if ignore images is false and it's from spectacle - we load the image in the cache and the history</li>
<li class="remarkup-list-item">if ignore images is false and it's not from spectacle - we load the image in the cache and the history</li>
<li class="remarkup-list-item">if ignoreImages is true and it's from spectacle, we load the image, but don't keep in history</li>
<li class="remarkup-list-item">if ignoreImages is true and it's not from spectacle we throw it away.</li>
</ol>

<p style="padding: 0; margin: 8px;">I think this patch is going to break case 2 as we now return early.</p>

<p style="padding: 0; margin: 8px;">But you're right that my comment was wrong, I meant.</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">if (m_ignoreImage && !data->
    return;</pre></div></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R120 Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D19852">https://phabricator.kde.org/D19852</a></div></div><br /><div><strong>To: </strong>kdautovic, Plasma: Workspaces, davidedmundson<br /><strong>Cc: </strong>ngraham, davidedmundson, plasma-devel, Plasma: Workspaces, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>