- User Since
- Apr 20 2015, 7:20 AM (252 w, 2 d)
Thu, Feb 13
Ok, I now fully got this, thanks for the explanation. And indeed without any left fill visual feedback is missing.
Wed, Feb 12
No offense meant, but even with the screenshots I still have no idea what this is about :)
Sun, Feb 9
I think this patch looks good.
Thu, Feb 6
I'd be fine with committing this. Still, there very clearly is large room to improve this further.
I like the idea of the patch, could you add a screenshot?
Thu, Jan 30
Better :) with corners I mean the 1-3 pixels left due to the rounding corners. These pixels were once also drawn as background although they are outside of the frame. It may be a minor detail, but imho such details are important. But indeed, the screenshots look good.
In two days we have the next KDE Frameworks tag. I'd have preferred a commit on Sunday ;)
Can you provide screenshots of more styles? :-)
Does that also work in Kate for floating messages like when the search wraps? What I want to know is whether the corners behind the green frame are transparent in this case, or whether the corners are painted solid. If I remember correctly, these kind of bugs were the reason to use Qt StyleSheets. And it must work with all styles.
Fri, Jan 24
Looks good to me, and very much in line with the other canConvert statements before.
Tue, Jan 21
Jan 20 2020
Looks good to me. Can you commit or shall we push this for you?
Jan 19 2020
Well, it definitely would be very nice if you find a patch that fixes this.
In that case I withdraw my review for now. I can only speak for Kate that I could not find a regression with my tests. But if you find other regressions, then this is likely not correct.
I just tested with KTextEditor's messagetest and KTextEditor's usage of KMessageWidget in the top and bottom bar, and all this looks still good.
Since KTextEditor uses KMessageWidget extensively we have to make sure the unit test messagetest still passes with this change.
Jan 16 2020
I guess this change is correct :-)
Jan 13 2020
That looks already better: If a line is wrapped and on the same page, then only the selected text is printed. It seems there are still corner cases.
- Create a document with just one long line that wraps over two printed pages. I this case, I am not able to print only the selected text properly.
- Say you have a line that wraps over e.g. 5 visual lines. And you just want to print the visually wrapped line 4 and 5. Right now, the visual lines 1-3 are just empty, but still take place. Maybe it makes sense to omit fully empty lines completely?
Jan 10 2020
Jan 6 2020
Jan 5 2020
Jan 3 2020
Jan 1 2020
Ok with this... Thanks. A note " Since 5.6x would be nice".
Dec 30 2019
Dec 29 2019
@hoffmannrobert: by the way, looking at your phabricator activity, you should get a KDE contributor account, if you don't have one already.
Dec 22 2019
@Gregor: That the terminal does not get focus on show was a regression in konsole part and should be fixed. I don't think there is any action item left for now.
Dec 15 2019
Dec 11 2019
Dec 10 2019
Was anything of the previously commented issues addressed?
Dec 8 2019
Doesn't this patch imply we have the same action twice now? Once provided by Konsole, and once by katemdi via View > Tool Views > Show Terminal?
Dec 6 2019
Dec 5 2019
Do we dislike iterators now?
We don't, and they still make sense for when you need the key, but range for is just much nier to look at :)
I'm fine with that statement. But are we going to be reviewing changing all the KDE code from iterators to range for? Feels like an overkill to me.
Looks good. But does the highlighting work for RW+CD? I am wondering whether + needs to be added to the weakDeliminator list?
Dec 4 2019
You can achieve Dolphin's behavior by reassigning the F4 shortcut to View > Tool Views > Show/Hide Konsole. That's what I do since > 10 years, and it works perfectly. Why is it not the default? Maybe because this action does not exist by default (i.e. when the Konsole plugin is not loaded). But the right fix is to reassign the F4 shortcut to this action.
Dec 3 2019
Dec 2 2019
Nov 26 2019
@jhayes Do you have commit access, or shall I commit this for you?
Any news here?
I think we should decide what to do with this patch, as over time it will get merge conflicts.
Nov 24 2019
I think this is ok.
Nov 23 2019
Makes sense, thanks.
Nov 22 2019
Nov 19 2019
Nov 16 2019
Can't you call rehighlight() yourself after calling setDefinition()?
You can force the current clang format to keep the multi-line if as follows:
Nov 11 2019
Nov 10 2019
@hein: Could you add a commit that relicenses all code to MIT? We switched to MIT for entire KSyntaxHighlighting. This will ease integration later.
Thanks, please commit! PS: Kate is also on gitlab: When pushing your branch to KDE's kate.git, the git push output will tell you how to create the merge request. But you can push directly as well, if you want.
Oct 31 2019
Also can't make it.
Oct 29 2019
Looks good to me.
Oct 28 2019
Hm right... too bad. I was hoping to find an automated way to detect this. Since relying on the user to optimize the RegExps will always be suboptimal. @cullmann Do you have any ideas?
Good, fine with me.
One option would be to add a capture or dontCapture attribute to enable or disable captures for RegExpr rules. Also, captures could be enabled or disabled in all RegExpr rules using the <general> group, adding an element for that.
Oct 27 2019
It's hard to review all the pixel changes with no screenshots. It's even unclear to me what problem exactly is fixed at hand. Given it's all your code, I trust you know what you are doing...
I wonder if the ?: optimizations make sense. QRegularExpression has the option QRegularExpression::DontCaptureOption to not capture anything. Looking into our code we have:
Oct 25 2019
Looks good to me, thanks!
Oct 20 2019
Imo we should simply try: We have another two weeks for testing.
Oct 19 2019
Oct 12 2019
@kossebau Just tried Okteta on Windows that is available on https://binary-factory.kde.org/job/Okteta_Nightly_win64/ and it works really well, out of the box. I believe it really has a lot of value making Okteta more visible, especially since users on Windows would also use it on Linux.
I'm all for it. This would unify how we can reformat any KDE module, which is very much desirable.
Please increase the version number and commit.
Thanks for the quick update.
Oct 10 2019
The External Tools plugin is done for now. I would like to wait a bit to get feedback, and over time we can certainly add more default tools, or even extend the plugin with additional functionality.
I believe work should be invested into the LSP client, since it provides an outline as well. Over time, the language servers will get better and better, so you'll get all this for free.