Differential D5735

Do not change clipboard while opening a searched file in KrSearchDialog
ClosedPublic
Actions

Authored by martinkostolny on May 7 2017, 1:14 AM.

Details

Reviewers
asensi
abika
Group Reviewers
Krusader
Commits
R167:f03636000f03: Add an option (persistent checkbox) in KrSearchDialog for adding a search query…
Summary

One can use a search dialog to search for files by text. A list of found files is shown and one can open such file by F3 or F4. In this case searched text is automatically set to clipboard. I imagine this can be useful. But personally I find it restrictive - most of the time I need my clipboard intact for other things. I have to re-set my clipboard every time I open a searched file.

This patch is just a proposal to remove this feature and leave user's clipboard alone.

I understand this can also be a handy feature so I'm prepared to make an option for it. In such case I suggest adding a persistent (survives restart) checkbox e.g. next to "profiles" button (bottom left) in search dialog. Any other suggestions? :)

Diff Detail

Repository
R167 Krusader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
martinkostolny created this revision.May 7 2017, 1:14 AM
asensi accepted this revision as: asensi.May 7 2017, 9:57 PM
asensi added a subscriber: asensi.

This patch is just a proposal to remove this feature and leave user's clipboard alone.

THANKS!

I understand this can also be a handy feature so I'm prepared to make an option for it.
In such case I suggest adding a persistent (survives restart) checkbox e.g. next to "profiles"
button (bottom left) in search dialog. Any other suggestions? :)

That's OK for me :-)

This revision is now accepted and ready to land.May 7 2017, 9:57 PM
abika added a subscriber: abika.May 7 2017, 10:12 PM

I vote for the option.
It was clearly intended as a feature and somebody is might using it. A checkbox can also advertise the feature to users who didn't even know it was there.

(And please combine the very same code and add a comment to it, so that new devs see that its there.)

martinkostolny updated this revision to Diff 14266.May 8 2017, 9:57 AM

Thanks for feedback! Here is an update with the option.

Alex, please check if I understood right the code combining and code comment.

One more thing: I've made a choice and disabled this feature by default. But I will gladly change that, if there will be votes against :).

abika accepted this revision as: abika.May 8 2017, 2:42 PM

Alex, please check if I understood right the code combining and code comment.

Great, thanks!

One more thing: I've made a choice and disabled this feature by default. But I will gladly change that, if there will be votes against :)

Fine by me. (And you don't have to do everything we say, its just our opinion like you have your own:)

Oh, and I wanted to mention that there are great clipboard managers like Parcellite for Linux - very useful. Only in case you didn't know.

Closed by commit R167:f03636000f03: Add an option (persistent checkbox) in KrSearchDialog for adding a search query… (authored by martinkostolny). · Explain WhyMay 8 2017, 7:07 PM
This revision was automatically updated to reflect the committed changes.
martinkostolny added a comment.May 8 2017, 7:22 PM

And you don't have to do everything we say.

I know :), but this time I really didn't care that much.

Now I'm not sure if I should open a review (and bother others) for every change I do. So far I've done that, but maybe I should only do that when a decision is to be made?

Parcellite for Linux - very useful. Only in case you didn't know.

Actually, I didn't know this one. And I really like it, thanks for the info!

Revision Contents
Changeset List

Diff 14307

View Options

krusader/Search/krsearchdialog.h

Loading...
View Options

krusader/Search/krsearchdialog.cpp

Loading...