> On May 15, 2015, 7:31 p.m., Martin Gräßlin wrote:
> > thanks for rebasing!
> > 
> > I just had a look at the bug report and have to agree with comment #1: I do 
> > from time to time copy on purpose whitespaces (yes I'm weird). I also tend 
> > to copy newlines and I do want to have them in the history. If I understand 
> > your commit description correctly this "feature" would break.
> > 
> > Given that I think we need more input on whether we want to break that 
> > feature or whether we want to create a config option for it.
> 
> Patrick Eigensatz wrote:
>     Hi Martin!
>     
>     Yes, this "feature" would break. However, if you copy more than one 
> whitespace sequence you won't be able to identify them in the klipper menu.
>     But your concerns are right. I could try to implement an option for this, 
> although I've never done this before and I would surely need some 
> assistance...
> 
> Kai Uwe Broulik wrote:
>     I also sometimes copy whitespace but then I immediately paste it in, so I 
> wouldn't need it in the history, its representation in the history could 
> probably be improved, if so desired. Perhaps add the usability group to this 
> review request so they can have a look at this.
> 
> Thomas Lübking wrote:
>     I usually copy whitespaces and newlines w/ the primary selection buffer 
> (for RMB) to click and paste commands when there's no WM ;-)
>     
>     As for distinguishing entries in the history, whitespaces could be 
> replaces w/ something printable like "·" or "?" resp. "?"?
>     Maybe in a different color to hint that this is a placeholder?
> 
> Patrick Eigensatz wrote:
>     I think the idea of (colored) placeholders is great! I've started to 
> create an option in the configuration dialog, we could add an option for 
> whitespace placeholders, too.

This is indeed one of the cases where having something configurable is a good 
idea. While hardly any "regular user" is likely to want to copy only 
whitespace, apparently this is an important feature for developers, so it 
should not be taken from them.

It should be turned off by default, however, because presumably developers are 
more likely to hunt for the option to turn it on than regular users are to hunt 
for the option to turn it off.

As for making whitespace placeholders configurable: That is too much. Either it 
is useful to have placeholders or it isn't, there isn't one group of people for 
whom it is useful and another for whom it isn't. Since the main usecase for 
copying only whitespace is in coding, where the actual number of whitespace 
characters often is relevant, it appears to make sense to have placeholders.
I would not use a printable character as a placeholder, though, because then it 
won't be distinguishable from the actual character. Can't we maybe just use a 
different color for the whitespaces (but only in cases where there are only 
whitespaces)?


- Thomas


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


On May 15, 2015, 7:42 p.m., Patrick Eigensatz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123806/
> -----------------------------------------------------------
> 
> (Updated May 15, 2015, 7:42 p.m.)
> 
> 
> Review request for kde-workspace, KDE Usability and Patrick Eigensatz.
> 
> 
> Bugs: 192922
>     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/historyitem.cpp 36cbe61 
> 
> Diff: https://git.reviewboard.kde.org/r/123806/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Patrick Eigensatz
> 
>

Reply via email to