[Differential] [Accepted] D4808: Find/Replace in files: fix tooltip omitting the matched text in Find mode

[Kevin Funk]

kfunk accepted this revision.
kfunk added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> kossebau wrote in grepoutputmodel.cpp:153
> The Grep toolview has two modes: Find-only mode and Replace mode.
> 
> The Find-only mode is the default mode and entered on doing the search.
> The Replace mode is triggered once one starts editing the "Replacement text" field.
> 
> In the Replace mode all items get changed to isCheckable = true, resulting in all items being shown with a checkbox, so one can select by toggling the checkboxes which of the found items will be included when doing the actual replacement.
> 
> So the isCheckable state reflects if in Find-only mode or Replace mode. The isChecked state only reflects if an item should be included in the replacement, it does not hint to the mode.
> 
> The bug is this:
> Currently the tooltip only cares about the Replacement mode, previewing the version of the item with the replacement done. Which in Find-only mode does not make sense, especially as the replacement text is empty and thus the tooltip shows only incomplete content. 
> Now one option would be to just not show the tooltip in Find mode. But it still serves a purpose there, when the matched line is much longer than what is shown in the view, so the tooltip allows to see the complete text (like done in similar tree views).
> So the fix here is to in Find mode (as reflected by the isCheckable == false) use the original text, not the replacement text, for the matched content in the line.
> 
> Any proposal how to make that more clear in the in-code comment?

Okay, understood, that's what I /thought/ as well.

My complaint is that using `isCheckable()` isn't going far enough. It should be `isCheckable() && isChecked()` -- since it only makes sense to show the to-be-replaced text when we actually want to replace it (IOW: when it is checked).

Anyhow, much better then before already: so go for it. Thanks!

REPOSITORY
  R33 KDevPlatform

BRANCH
  smallfixestogrepview

REVISION DETAIL
  https://phabricator.kde.org/D4808

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kossebau, #kdevelop, kfunk
Cc: kfunk, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170227/4db89526/attachment.html>