> On May 17, 2015, 8: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.
> 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. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123806/#review80532 ----------------------------------------------------------- On May 16, 2015, 11:31 p.m., Patrick Eigensatz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123806/ > ----------------------------------------------------------- > > (Updated May 16, 2015, 11:31 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 > >
