> 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. > > Thomas Pfeiffer wrote: > 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)? > > Kai Uwe Broulik wrote: > We could enable styledText for such cases and then use arbitrary html > formatting for colors. However, I think there are even Unicode characters > that look like blocks with "TAB" and similar written in them. > > Thomas Pfeiffer wrote: > I'm fine with anything that isn't a regular character. > > Patrick Eigensatz wrote: > I remember there is a programming language called "Whitespace" only > consisting of space/tab/newline. Whitespace IDEs highlight those chars > somehow. I think we might change the background color of a char, for example > spaces are "marked" green and so on... (Using HTML formatting? I'm not sure > what is possible here...) (The point to do this *only* where there are only > whitespaces is important!) > > I'm currently trying to develop the configuration setting "Ignore > whitespace characters" to but I'm new to Qt and KDE development in general.
I'd call it "Ignore selections that contain only whitespace", because we're not ignoring whitespaces completely. - 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 > >
