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

[Patrick Eigensatz]

> On Mai 17, 2015, 6:22 nachm., 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.

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


- Patrick


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


On Mai 16, 2015, 9:31 nachm., Patrick Eigensatz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123806/
> -----------------------------------------------------------
> 
> (Updated Mai 16, 2015, 9:31 nachm.)
> 
> 
> 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/ba1ff319/attachment.htm>