<table><tr><td style="">rjvbb 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/D18380">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><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);">I didn't read the full encyclopedia of discussions here, I only looked at the patch.</pre></div></blockquote>

<p>It's really a pity that you only did that now, because by now I'll have to reverse-engineer my patch (and read the encyclopedia) to remember how I arrived at the patch.</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><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);">But really, this is far too hacky to my taste. If QHeaderview is indeed missing a feature, it might be best to implement it there, it will be much cleaner.</pre></div></blockquote>

<p>Yes, it has a good perfume of a proper hack, I won't disagree with that. But sometimes that's unavoidable if the cleaner solution is by definition a long-term one (that's unlikely to appear in the current Qt LTS version, for instance).</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>This is a horrible hack, relying on the exact number of parents before getting to KFileWidget.</p></blockquote>

<p>I'm not sure I agree. IIRC it's based on knowledge of how the standard KIO filedialog is constructed, which seems OK in another part of KIO code.</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>Either cast inside a while loop (i.e. "go up until a KFileWidget"), or find a way to pass the proper pointer to this class.</p></blockquote>

<p>You'd probably want to put an additional end condition to that loop, no?</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>Why is this (and the following 12 lines or so) done on every resize? Surely that's not needed?</p></blockquote>

<p>If memory serves me well it may not be needed on every loop indeed, but it is also not UNneeded on any loop (i.e. sometimes it is).</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>URGGH. This smells like a horrible hack. Widgets do layouting, and then painting. Modifying layouting parameters (hiding columns, resizing columns...) at painting time is very wrong and a recipe for infinite loops (paint -> layout -> paint -> layout...).</p></blockquote>

<p>Maybe, but apparently not for the last months in my own test kitchen ...</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>Isn't that what Polish is for?</p></blockquote>

<p>I am tempted to say that I should have seen polish events arrive at an appropriate time during the mentioned event analysis. I'm not unfamiliar with those thanks to my work on QtCurve, so I'd hope that it would have occurred to me to exploit them.</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>If you mean Q_ASSERT, use Q_ASSERT. If on the other hand, it can happen for good reasons, remove the qWarning.</p></blockquote>

<p>No, I most definitely didn't mean Q_ASSERT. I won't ever knowingly use anything that leads to a crash/abort if there's even a chance that the situation can be handled more gracefully.</p>

<p>All in all I notice I have lost much if not all of my appetite to do much more on this beyond the couple of simpler suggestions made here... I haven't even found the time to update my installs from 5.52.0 (and am still on Qt 5.9 too, at that (though with some stuff backported from 5.10).</p>

<p>Anyone wanting to take over ("commandeer") this patch, please feel free!!</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D18380">https://phabricator.kde.org/D18380</a></div></div><br /><div><strong>To: </strong>rjvbb, ngraham, Frameworks, Dolphin, apol, dfaure, ahartmetz, markg<br /><strong>Cc: </strong>markg, cfeck, dhaumann, kwrite-devel, kde-frameworks-devel, michaelh, ngraham, bruns<br /></div>