<table><tr><td style="">ahmadsamir marked an inline comment as done.<br />ahmadsamir 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/D28675">View Revision</a></tr></table><br /><div><div><p>I thought that adding the "comment" as a tooltip makes sure it's always available if the dialog is shown with the flag for the comment column unset... but looking on lxr.kde all the usage of KMimeTypeChooserDialog always show the comments column; so that idea is redundant.</p>
<p>A tooltip isn't good enough here? I think the mimetype name and extension are more recognisable/useful than the comment; especially since the comment sometimes isn't that useful:<br />
AMR AMR Audio<br />
troff Troff document<br />
x-base32 Base32 encoded data<br />
x-rpm-spec RPM spec file</p>
<p>but I haven't set in lines in the sand about that column, bringing it back should be easy.</p>
<blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D28675#645714" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D28675#645714</a>, <a href="https://phabricator.kde.org/p/dfaure/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@dfaure</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>I'm also curious about the use of QStandardItemModel which I consider to be at best a class for unittesting proxies and views. Just like QTreeWidgetItem it stores the data.<br />
Of course compared to QTreeWidgetItem it's one step into the "right" direction of item/view separation, so why not, but it's not exactly more efficient.</p></div>
</blockquote>
<p>That answers a question that kept popping in my mind while working on this, whether to try sub-class'ing QAbstractItemModel, or just use QStandardItemModel; basically whether one is better than the other. Certainly the documentation that I read so far doesn't tell me exactly which way to go. Of course I noticed lots of sub-classing of QAbstractItemModel in KDE, but it seemed easier (less work :)) to use an existing model rather than sub-classing.</p>
<p>So what you're saying here is that sub-classing QAbstractItemModel is better; I'll try that, but I am curious about why a sub-class is better than using QStandardItemModel (are there any docs/blogs... etc about that?)</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/D28675#inline-163700">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kmimetypechooser.cpp:44</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">In my experience a vector of const pointers is usually a pain to work with, e.g. for find(). But you're lucky here it seems, it doesn't create trouble ;)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">A vector of non-const pointers is better in that regard?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R236 KWidgetsAddons</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28675">https://phabricator.kde.org/D28675</a></div></div><br /><div><strong>To: </strong>ahmadsamir, Frameworks, cfeck, dfaure<br /><strong>Cc: </strong>kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns<br /></div>