<table><tr><td style="">dhaumann requested changes to this revision.<br />dhaumann added a comment.<br />This revision now requires changes to proceed.
</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/D4847" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>This is a good patch.</p>
<p>My main concerns are:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">please use const for variables that do not change anymore</li>
<li class="remarkup-list-item">please don't use const & for primitive data types (like bool) in function arguments, that does not make sense</li>
<li class="remarkup-list-item">naming conventions: Instead of TextBufferSecure, the word SecureTextBuffer sounds much more natural. Similarly, the header file can be renamed.</li>
</ul>
<p>Could you provide an updated revision?</p>
<p>Some general questions</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">Does that affect other platforms such as Windows in any way? I.e., does KAuth exist on Windows?</li>
<li class="remarkup-list-item">The text buffer itself is unit tested pretty well. Is it possible to have a unit test for this? PS: We already have a " bool KTextEditor::EditorPrivate::unitTestMode()" function that we could use to virtually trigger the KAuth case somehow, not sure.</li>
</ul></div></div><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/D4847#inline-19757" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer.cpp:768-769</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span class="bright"></span><span style="color: #304a96"><span class="bright">#ifndef Q_OS_WIN</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="bright"></span><span style="color: #aa4000"><span class="bright">const</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">bool</span></span><span class="bright"> </span><span class="n"><span class="bright">newFile</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">!</span></span><span class="bright"></span><span class="n"><span class="bright">QFile</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">exists</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">file</span>name</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span class="bright"></span><span style="color: #304a96"><span class="bright">#endif</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #74777d">/**</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span style="color: #74777d"> * use QSaveFile for save write + rename</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">QSaveFile</span> <span style="color: #004012">saveFile</span><span class="p">(</span><span class="n">filename</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">saveFile</span><span class="p">.</span><span class="n">setDirectWriteFallback</span><span class="p">(</span><span style="color: #304a96">true</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">saveFile</span><span class="p">.</span><span class="n">open</span><span class="p">(</span><span class="n">QIODevice</span><span style="color: #aa2211">::</span><span class="n">WriteOnly</span> <span style="color: #aa2211">|</span> <span class="n">QIODevice</span><span style="color: #aa2211">::</span><span class="n">Truncate</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #aa4000">return</span> <span style="color: #304a96">false</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #74777d">/**</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span style="color: #74777d"> * construct correct filter device and try to open</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">KCompressionDevice</span><span style="color: #aa2211">::</span><span class="n">CompressionType</span> <span class="n">type</span> <span style="color: #aa2211">=</span> <span class="n">KFilterDev</span><span style="color: #aa2211">::</span><span class="n">compressionTypeForMimeType</span><span class="p">(</span><span class="n">m_mimeTypeForFilterDev</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">KCompressionDevice</span> <span style="color: #004012">file</span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">saveFile</span><span class="p">,</span> <span style="color: #304a96">false</span><span class="p">,</span> <span class="n">type</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span style="color: #aa2211">!</span><span class="n">file</span><span class="p">.</span><span class="n">open</span><span class="p">(</span><span class="n">QIODevice</span><span style="color: #aa2211">::</span><span class="n">WriteOnly</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #aa4000">return</span> <span style="color: #304a96">false</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #74777d">/**</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span style="color: #74777d"> * construct stream + disable Unicode headers</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">QTextStream</span> <span class="n">stream</span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">file</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">stream</span><span class="p">.</span><span class="n">setCodec</span><span class="p">(</span><span class="n">QTextCodec</span><span style="color: #aa2211">::</span><span class="n">codecForName</span><span class="p">(</span><span style="color: #766510">"UTF-16"</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #74777d">// set the correct codec</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">stream</span><span class="p">.</span><span class="n">setCodec</span><span class="p">(</span><span class="n">m_textCodec</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #74777d">// generate byte order mark?</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">stream</span><span class="p">.</span><span class="n">setGenerateByteOrderMark</span><span class="p">(</span><span class="n">generateByteOrderMark</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #74777d">// our loved eol string ;)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">QString</span> <span class="n">eol</span> <span style="color: #aa2211">=</span> <span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span><span class="p">);</span> <span style="color: #74777d">//m_doc->config()->eolString ();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">endOfLineMode</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">eolDos</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">eol</span> <span style="color: #aa2211">=</span> <span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"</span><span style="color: #bb6622">\r\n</span><span style="color: #766510">"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="p">}</span> <span style="color: #aa4000">else</span> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">endOfLineMode</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">eolMac</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="n">eol</span> <span style="color: #aa2211">=</span> <span class="n">QStringLiteral</span><span class="p">(</span><span style="color: #766510">"</span><span style="color: #bb6622">\r</span><span style="color: #766510">"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span style="color: #74777d">// just dump the lines out ;)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="bright"> </span><span style="color: #74777d"><span class="bright">// prepare parameters for calling helper method</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="bright"></span><span class="n"><span class="bright">QString</span></span><span class="bright"> </span><span class="n"><span class="bright">textCodec</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span class="n"><span class="bright">QString</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">fromLatin1</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">m_textCodec</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="n">name<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">()</span>);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="bright"> </span><span style="color: #aa4000"><span class="bright">int</span></span><span class="bright"> </span><span class="n"><span class="bright">eolMode</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">static_cast</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><</span></span><span class="bright"></span><span style="color: #aa4000"><span class="bright">int</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">></span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">endOfLineMode</span></span><span class="bright"></span><span class="p"><span class="bright">());</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="bright"> </span><span class="n"><span class="bright">QList</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><</span></span><span class="bright"></span><span class="n"><span class="bright">QVariant</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">></span></span><span class="bright"> </span><span class="n"><span class="bright">dataToSave</span></span><span class="bright"></span><span class="p"><span class="bright">;</span></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">const QString textCodec = ...<br />
const int eolMode = ...</p>
<p style="padding: 0; margin: 8px;">Please use const whenever possible for locally defined variables.</p></div></div><br /><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/D4847#inline-19758" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.cpp:1</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #304a96">#include</span> <span class="cpf">"katetextbuffer_secure.h"</span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #304a96">#include</span> <span class="cpf">"katetextline.h"</span><span style="color: #304a96"></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I would prefer katesecuretextbuffer.h as filename. Intermixing underscrores is rather uncommon in KDE's sources.</p></div></div><br /><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/D4847#inline-19763" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.cpp:26-31</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QString</span> <span class="n">filename</span> <span style="color: #aa2211">=</span> <span class="n">args</span><span class="p">[</span><span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"filename"</span><span class="p">)].</span><span class="n">toString</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QString</span> <span class="n">mimeTypeForFilterDev</span> <span style="color: #aa2211">=</span> <span class="n">args</span><span class="p">[</span><span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"mimeTypeForFilterDev"</span><span class="p">)].</span><span class="n">toString</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QString</span> <span class="n">textCodec</span> <span style="color: #aa2211">=</span> <span class="n">args</span><span class="p">[</span><span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"textCodec"</span><span class="p">)].</span><span class="n">toString</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">bool</span> <span class="n">generateByteOrderMark</span> <span style="color: #aa2211">=</span> <span class="n">args</span><span class="p">[</span><span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"generateByteOrderMark"</span><span class="p">)].</span><span class="n">toBool</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">int</span> <span class="n">endOfLineMode</span> <span style="color: #aa2211">=</span> <span class="n">args</span><span class="p">[</span><span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"endOfLineMode"</span><span class="p">)].</span><span class="n">toInt</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">bool</span> <span class="n">newLineAtEof</span> <span style="color: #aa2211">=</span> <span class="n">args</span><span class="p">[</span><span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"newLineAtEof"</span><span class="p">)].</span><span class="n">toBool</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please prepend 'const' for all local variables whenever possible.</p></div></div><br /><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/D4847#inline-19764" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.cpp:34</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">bool</span> <span class="n">ok</span> <span style="color: #aa2211">=</span> <span class="n">saveInternal</span><span class="p">(</span><span class="n">filename</span><span class="p">,</span> <span class="n">mimeTypeForFilterDev</span><span class="p">,</span> <span class="n">textCodec</span><span class="p">,</span> <span class="n">generateByteOrderMark</span><span class="p">,</span> <span class="n">endOfLineMode</span><span class="p">,</span> <span class="n">newLineAtEof</span><span class="p">,</span> <span class="n">dataToSave</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">const bool ok = ...</p></div></div><br /><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/D4847#inline-19766" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.cpp:87</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #74777d">// just dump the lines out ;)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">int</span> <span class="n">lineCount</span> <span style="color: #aa2211">=</span> <span class="n">dataToSave</span><span class="p">.</span><span class="n">count</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">for</span> <span class="p">(</span><span style="color: #aa4000">int</span> <span class="n">i</span> <span style="color: #aa2211">=</span> <span style="color: #601200">0</span><span class="p">;</span> <span class="n">i</span> <span style="color: #aa2211"><</span> <span class="n">lineCount</span><span class="p">;</span> <span style="color: #aa2211">++</span><span class="n">i</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">const int lineCount = ...</p></div></div><br /><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/D4847#inline-19762" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.h:10</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="n">using</span> <span class="n">namespace</span> <span class="n">KAuth</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please no using namespace KAuth here, especially since this is a header file.</p></div></div><br /><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/D4847#inline-19761" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.h:12</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="n">class</span> <span style="color: #a0a000">TextBufferSecure</span> <span class="p">:</span> <span class="n">public</span> <span class="n">QObject</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please call it SecureTextBuffer, that feels much more natural than adding the suffix "Secure".</p></div></div><br /><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/D4847#inline-19760" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.h:18</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">TextBufferSecure</span><span class="p">()</span> <span class="p">{};</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Pedantic: no semicolon after a closing function brace, i.e.:</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);">SecureTextBuffer() {}</pre></div></div></div><br /><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/D4847#inline-19759" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katetextbuffer_secure.h:22</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">bool</span> <span style="color: #004012">saveInternal</span><span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">QString</span> <span style="color: #aa2211">&</span><span class="n">filename</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QString</span> <span style="color: #aa2211">&</span><span class="n">mimeTypeForFilterDev</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QString</span> <span style="color: #aa2211">&</span><span class="n">textCodec</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span style="color: #aa4000">bool</span> <span style="color: #aa2211">&</span><span class="n">generateByteOrderMark</span><span class="p">,</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">int</span> <span style="color: #aa2211">&</span><span class="n">endOfLineMode</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span style="color: #aa4000">bool</span> <span style="color: #aa2211">&</span><span class="n">newLineAtEof</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QList</span><span style="color: #aa2211"><</span><span class="n">QVariant</span><span style="color: #aa2211">></span> <span style="color: #aa2211">&</span><span class="n">dataToSave</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please never use const references for primitive types. Here:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">const bool & --> bool</li>
</ul></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R39 KTextEditor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D4847" rel="noreferrer">https://phabricator.kde.org/D4847</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>martinkostolny, KTextEditor, dhaumann<br /><strong>Cc: </strong>dhaumann, graesslin, davidedmundson, palant, kwrite-devel, Frameworks, head7, cullmann, kfunk, sars<br /></div>