<table><tr><td style="">dfaure added a comment.
</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/D28221">View Revision</a></tr></table><br /><div><div><p>Ah! Now it actually makes sense to me. If we are changing what revertToDefault() does, then it makes sense to change the if() condition for calling it. Basically, now that it does the right thing in both cases (default from C++ and default from system file) it can be called in both cases, while before it was only called if there was no default from a system file. Right?</p>
<p>I'm still wondering about this though: "If we're saving a value equal to the C++ default, then we have to write it out, otherwise upon reading the global-file default will interfer."<br />
Your unittest shows that revertToDefault() will lead to the global-file value being read.<br />
Doesn't this mean that this code is wrong then?</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 (value == "cppdefault") {
cg.revertToDefault(key);
} else {
cg.writeEntry(key, value);
}</pre></div>
<p>That's the code you're using everywhere, so that's actually the code that would be good to unittest ;-)<br />
Isn't this buggy when the value actually *is* cppdefault, and there is a system config file with another default value?</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Indeed probably need to update configuration too.</p></blockquote>
<p>Did you mean documentation? I know this is all about configuration ;) but the bit that's missing is to update the documentation, and add a task to get rid of the hasDefault() everywhere before revertToDefault is called. Assuming I'm wrong above about there being a bug left :-)</p></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/D28221#inline-166794">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kconfigtest.cpp:1991</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: rgba(151, 234, 151, .6);"> <span class="n">KConfigGroup</span> <span style="color: #004012">generalLocal</span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">local</span><span class="p">,</span> <span style="color: #766510">"General"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// Check if we get global and not the default value from cpp (defaultcpp) when reading data from restorerc</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QCOMPARE</span><span class="p">(</span><span class="n">generalLocal</span><span class="p">.</span><span class="n">readEntry</span><span class="p">(</span><span style="color: #766510">"testKeyDefault"</span><span class="p">,</span> <span style="color: #766510">"defaultcpp"</span><span class="p">),</span> <span style="color: #766510">"configdefault"</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Bug in the comment, the file isn't called restorerc.</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/D28221#inline-166795">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kconfigtest.cpp:1998</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: rgba(151, 234, 151, .6);"> <span class="n">local</span><span class="p">.</span><span class="n">reparseConfiguration</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// We expect to get configdefault value and configdefault not wrote to local file (file will not exist)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QCOMPARE</span><span class="p">(</span><span class="n">generalLocal</span><span class="p">.</span><span class="n">readEntry</span><span class="p">(</span><span style="color: #766510">"testKeyDefault"</span><span class="p">,</span> <span style="color: #766510">"defaultcpp"</span><span class="p">),</span> <span style="color: #766510">"configdefault"</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">s/wrote/written/</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/D28221#inline-166796">View Inline</a><span style="color: #4b4d51; font-weight: bold;">kconfigtest.cpp:2002</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: rgba(151, 234, 151, .6);"> <span style="color: #74777d">// Write a custom value, we expect this value to be wrote to local file</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">generalLocal</span><span class="p">.</span><span class="n">writeEntry</span><span class="p">(</span><span style="color: #766510">"testKeyDefault"</span><span class="p">,</span> <span style="color: #766510">"custom"</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">s/wrote/written/</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R237 KConfig</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28221">https://phabricator.kde.org/D28221</a></div></div><br /><div><strong>To: </strong>bport, ervin, dfaure, davidedmundson<br /><strong>Cc: </strong>kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns<br /></div>