Review Request 123806: [klipper] Ignore empty / blank entries

[Thomas Pfeiffer]

> On May 17, 2015, 6:22 p.m., Martin Gräßlin wrote:
> > klipper/generalconfig.ui, lines 36-45
> > <https://git.reviewboard.kde.org/r/123806/diff/5/?file=369604#file369604line36>
> >
> >     unrelated to the discussion about how to call the entries: I would call the config option differently as the code is not trimming whitespaces but rather ignoring whitespace entries.
> 
> Patrick Eigensatz wrote:
>     Are we trimming whitespace? Or are we going to simplify it? I try to call the option as sensible/useful as possible.
>     
>     It seems as if we're going to simplify the string if the option is enabled, so the option might be labeled "Intelligent Whitespace Handling" or something like this and inside the code we might call it *kcfg_SimplifyEntries*. Is this okay? I'll start implementing this as soon as I get
>     green light from you. ;)
> 
> Martin Gräßlin wrote:
>     I'd call the config option like what it does. Simplifiy is generic - it could be anything. I'd call it something like "kcfg_ignoreEmptyEntries". What I mostly care about is how later on the code looks like. It should be easy to understand why the isEmpty() string is ignored. With a name like "simplifyEntries" that's not obvious.
> 
> Patrick Eigensatz wrote:
>     I'm sorry I'm still not sure whether you want me to imlement only a simplify function or a simplify function AND an ignore empty entry function or only an ignore function. (See comment above)
>     Thank you
> 
> Martin Gräßlin wrote:
>     I'm for "ignore empty entry" function. I think simplify is a bad idea cornering black magic ;-)
> 
> Thomas Lübking wrote:
>     +1 for unconditional indicating of whitespaces
>     +1 for NOT manipulating (includes ignoring) the buffer by default (klipper would change the normal behavior of selection buffers by default)
>     
>     About simplifications: since pasting newlines w/ or w/o klipper into a shell interpreter is problematic, that could be deemed the shells job. (Afair konsole even has/had a feature to suppress this)
> 
> Patrick Eigensatz wrote:
>     Okay,I'm going to implement this feature this evening. 
>     
>     @ Thomas: So you think it should be allowed by default? (whitespaces)
> 
> Thomas Lübking wrote:
>     Yupp.
>     
>     The reason is that (atm.) klipper "enhances" the default selection/clipboard buffers (No more: "why does cnp no longer work after closing windows" bug reports) but otherwise is in stealth mode (by default)
>     
>     If we have some "secret" (I don't even have a systray icon for it) daemon manipulating the "normal" behavior ("do as I say"), by restricting it to "i do what i believe is good for you", we'll likely get angry bug reports =)
>     
>     
>     
>     If I copy whitespace and paste whitespace and am "confused" about that (the system did what I asked it to do), that's seriously my problem (easily confused, maybe dumb).
>     
>     If i'm not confused, I still might indeed want a "smarter" behavior, so it's good to offer that.
>     
>     If I copy whitespace and paste "my boss is a tool", I'd perceive that as a bug (and get fired ;-) because the system does not do what I asked it to do.
>     
>     A trade-off for the default behavior could be to copy whitespace, but not keep add it to the history by default (cause that's a self-restriction of a klipper feature only)
> 
> Patrick Eigensatz wrote:
>     Ahhh, I see... I'm sorry, I probably mis-explained my stuff :/
>     I never talked about modifying the actual clipboard, I only thought about not adding it to the history.  (Sorry, I was not that evident!) 
>     
>     As you said, a "secret" system modifying the clipboard in the background is not smart all, especially if the users don't know why clipboard won't change if they try to
>     copy a few whitespaces.
>     
>     I thaught about the option as "Allow only-whitespace-entries" (in history) which would be false by default, but for example developers who are often copying whitespaces could activate the
>     option, so klipper also saves those in history.
> 
> Martin Gräßlin wrote:
>     > I thaught about the option as "Allow only-whitespace-entries" (in history) which would be false by default,
>     
>     Yes, I think that's a good solution.

Okay, let's do it like that then.
For the label I'd suggest "Include whitespace-only entries in the history".


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123806/#review80532
-----------------------------------------------------------


On May 18, 2015, 7:25 p.m., Patrick Eigensatz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123806/
> -----------------------------------------------------------
> 
> (Updated May 18, 2015, 7:25 p.m.)
> 
> 
> Review request for kde-workspace, KDE Usability and Patrick Eigensatz.
> 
> 
> Bugs: 159267 and 192922
>     https://bugs.kde.org/show_bug.cgi?id=159267
>     https://bugs.kde.org/show_bug.cgi?id=192922
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> [PATCH] plasma-workspace: klipper: Fix #192922 Ignore blank entries
> 
> QString::isEmpty() is used to check if the string only consists of whitespace characters. If it does, the creation of the HistoryStringItem fails.
> 
> 
> Diffs
> -----
> 
>   klipper/generalconfig.ui f513e9c 
>   klipper/historyitem.cpp 36cbe61 
>   klipper/klipper.h 6952b11 
>   klipper/klipper.cpp 798b49f 
>   klipper/klipper.kcfg a03dd16 
> 
> Diff: https://git.reviewboard.kde.org/r/123806/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Patrick Eigensatz
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20150518/79898bdd/attachment.htm>