<table><tr><td style="">simgunz marked an inline comment as done.<br />simgunz 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/D8056" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p><a href="https://phabricator.kde.org/p/rkflx/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@rkflx</a> I agree on all your points.</p>
<p>In general I also prefer to have the categories. Implementing what you propose will require a bit of time. Basically if we want any behavior different from the current one, we need to replace the <tt style="background: #ebebeb; font-size: 13px;">QTreeView</tt> with a <tt style="background: #ebebeb; font-size: 13px;">QListView</tt> every time some text is typed in the <tt style="background: #ebebeb; font-size: 13px;">QLineEdit</tt>.</p>
<p>I agree in opening a new task to talk about the proposed points.</p>
<p>For this patch everything should be ok, now. I also fixed the <tt style="background: #ebebeb; font-size: 13px;">QRegExp</tt> point.</p>
<p>Just for my mental sanity, what is the remarkup syntax to make the ESC key? key Esc or Escape shows a hammer and sickle :-)</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/D8056#inline-38149" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kopenwithdialog.cpp:826</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;"><a href="https://phabricator.kde.org/p/simgunz/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;" rel="noreferrer">@simgunz</a> This is marked as done, but still uses <tt style="background: #ebebeb; font-size: 13px;">QRegExp</tt> and not <tt style="background: #ebebeb; font-size: 13px;">QRegularExpression</tt>?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I was using the regular expression only to provide cases insensitive filter, but I saw now I can achieve it with fixed string filter as well.</p>
<p style="padding: 0; margin: 8px;">The interface of the method <tt style="background: #ebebeb; font-size: 13px;">setFilterRegExp</tt> only accepts <tt style="background: #ebebeb; font-size: 13px;">QRegExp</tt>.</p>
<p style="padding: 0; margin: 8px;">Thanks for pointing this out.</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/D8056#inline-38153" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">rkflx</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kopenwithdialog.cpp:1099</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Is this a "fixme-before-commit" or a "fixme-sometimes-in-the-future" FIXME?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">It is a fixme-sometimes-in-the-future. Actually I am not even sure how much of a problem that is, so any other opinion is welcomed.</p>
<p style="padding: 0; margin: 8px;">To be more clear on this point. Currently <kbd title="Down" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↓</kbd> moves the focus to the tree view, while <kbd title="Up" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↑</kbd> moves it back in the line edit history of commands. When the search provides no results, <kbd title="Down" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↓</kbd> moves forward in the history. This is true for any completion type.</p>
<p style="padding: 0; margin: 8px;">Before my changes, <kbd title="Down" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↓</kbd> was just moving forward in the history of commands.</p>
<p style="padding: 0; margin: 8px;">When we use CompletionPopup or CompletionPopupAuto we want <kbd title="Down" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↓</kbd> to let use navigate the entries in the popup, so it shouldn't be used to focus on the treeview. At this point there are two possibilities:</p>
<ol class="remarkup-list">
<li class="remarkup-list-item">We hit <kbd style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">Enter</kbd> and select an entry. The popup closes, and in the tree there will be likely a single result if the command chosen has also a desktop entry, or no results if the command is a shell command</li>
<li class="remarkup-list-item">We hit <tt style="background: #ebebeb; font-size: 13px;">Esc</tt> and in the line edit there is a piece of text that may match something in the tree view</li>
</ol>
<p style="padding: 0; margin: 8px;">At this point, once the popup is closed, one could use <kbd title="Down" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↓</kbd> to focus the tree view, but currently <kbd title="Down" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↓</kbd> is disabled. So in this sense the behavior is not consistent with the cases where other completion modes are used. (Note that when the popup is closed <kbd title="Down" style="display: inline-block; min-width: 1em; padding: 4px 5px 5px; font-weight: normal; font-size: 0.8rem; text-align: center; text-decoration: none; line-height: 0.6rem; border-radius: 3px; box-shadow: inset 0 -1px 0 rgba(71, 87, 120, 0.08); user-select: none; background: #f7f7f7; border: 1px solid #C7CCD9;">↓</kbd> does not open it again, only typing more text opens it)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R241 KIO</div></div></div><br /><div><strong>BRANCH</strong><div><div>openwithdialog-filter-app-tree</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8056" rel="noreferrer">https://phabricator.kde.org/D8056</a></div></div><br /><div><strong>To: </strong>simgunz, dfaure, Frameworks, VDG, ngraham<br /><strong>Cc: </strong>rkflx, subdiff, fabianr, abetts, ngraham, alexeymin, Frameworks<br /></div>