> On May 15, 2015, 1: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. > > Thomas Pfeiffer wrote: > I'd call it "Ignore selections that contain only whitespace", because > we're not ignoring whitespaces completely. > > Thomas Lübking wrote: > > I'm fine with anything that isn't a regular character. > > Oh, cool - RB screwed it. > > Run kcharselect and select "Symbole" and "Symbole für Steuerzeichen" > (anybody around not german? =) > There're special glyphs for non-printable characters. I added them to my > former post, but RB apparently replaced them w/ nothing :-( > > eg. U+2424 (newline) and U+2420 (space) > > Patrick Eigensatz wrote: > You're right on this one! ;-) > > Thomas Pfeiffer wrote: > Okay, Patrick (or anyone else who can run Master), I'd like you to do > something called "guerilla usability testing": Once the placeholder feature > has been implemented, please do the following: > Take a machine with Klipper open and a line with the placeholders in it > to a few people who > - are used to coding or writing a markup language > - ideally are at least somewhat familiar with Klipper > - are not not involved in this review request > and ask them what they think they see there. > If all of them at least _guess_ it's whitespace, we've won. If some of > them think it's something else, we have to go back to the drawing board. > Unicode actually gives us a few options here. In addition to the > characters Thomas mentioned, we also have U+2423 (open box, graphic for > space) and U+2422 (blank symbol, graphic for space). And if all of those > fail, we can still try colors. > > Could you do that? We don't want to introduce something which only we > think works, so this would be really helpful. It's also interesting/fun to do > and doesn't cost much time. > Thanks! > > Patrick Eigensatz wrote: > Hi Thomas! > > Indeed! The test seems to be fun! U+2423 seems almost perfect for spaces! > But as I told you, I'm new to Qt and I'm new to KDE development and I'm > not sure > if I can write all this code alone, but I hope to do so... > > Patrick Eigensatz wrote: > I already run into some troubles reading the code. Inside the *Klipper* > class, there seems to be the method *loadSettings()* respectively > *saveSettings()*. > They use functions from a namespace called *KlipperSettings*. But this > namespace is nowhere defined. Often, a file called "klippersettings.h" is > included, > but in this git commit, this file does not exist... > > Thank you for your help :)
Patrick, First off, welcome to the community! To answer your question though, the klippersettings.h and it's matching .cpp file are autogenerated files from the .kcfg file. More information can be found here: https://techbase.kde.org/Development/Tutorials/Using_KConfig_XT You should be able to find them in your build folder somewhere. - Jeremy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123806/#review80423 ----------------------------------------------------------- On May 15, 2015, 1: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, 1: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 > >
