Re: Review Request 117695: change where dynamic replace tabs is performed

2014-04-22 Thread Sven Brauch
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117695/#review56220 --- On April 22, 2014, 9:06 p.m., Sven Brauch wrote

Re: Review Request 117695: change where dynamic replace tabs is performed

2014-04-23 Thread Sven Brauch
546d3e6aadc57f24c3fa766ce235addc0f02e3c3 Diff: https://git.reviewboard.kde.org/r/117695/diff/ Testing --- Just some quick manual tests, it seems to still work as intended. Thanks, Sven Brauch ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https

Re: Review Request 117695: change where dynamic replace tabs is performed

2014-04-23 Thread Sven Brauch
quick manual tests, it seems to still work as intended. Thanks, Sven Brauch ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 117695: change where dynamic replace tabs is performed

2014-04-24 Thread Sven Brauch
/#review56439 --- On April 23, 2014, 11:15 a.m., Sven Brauch wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117695

Re: Review Request 117695: change where dynamic replace tabs is performed

2014-05-02 Thread Sven Brauch
Diff: https://git.reviewboard.kde.org/r/117695/diff/ Testing --- Just some quick manual tests, it seems to still work as intended. Thanks, Sven Brauch ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org

Re: Review Request 117695: change where dynamic replace tabs is performed

2014-05-07 Thread Sven Brauch
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117695/#review57494 --- Ping, any new opinions on this? - Sven Brauch On May 3

Re: Review Request 117695: change where dynamic replace tabs is performed

2014-05-19 Thread Sven Brauch
/diff/ Testing --- Just some quick manual tests, it seems to still work as intended. Thanks, Sven Brauch ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Supporting MSVC2010 in ktexteditor framework

2014-11-05 Thread Sven Brauch
Actually I just found it doesn't build with VS2012 either, ugh. And I expected the fixes to be easier, but it seems I still have to fix the unit test full of brace initializers for 2012 to work. I will fix that, after all it's my fault it looks like this. Just tell me what features I can't use.

Re: Review Request 122330: Associate *.qmltypes and *.qmlproject files with the text/x-qml mime type

2015-01-30 Thread Sven Brauch
those files into a new mime type and add that to the KDevelop plugin as well (it supports specifying multiple mime types already), but since the files are valid QML syntax, I don't see a need for that. - Sven Brauch On Jan. 30, 2015, 2:48 p.m., Denis Steckelmacher wrote

Re: Review Request 126212: Do not crash if KDE platform integration is loaded but SNI is unavailable

2015-12-07 Thread Sven Brauch
marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit b45544f3d4dd9cb1873b92a609f45a68f5f6e471 by Sven Brauch to branch master. Bugs: 350785 https://bugs.kde.org/show_bug.cgi?id=350785 Repository: knotifications Description --- See bug

Review Request 126212: Do not crash if KDE platform integration is loaded but SNI is unavailable

2015-11-30 Thread Sven Brauch
/kstatusnotifieritem.cpp c48f4d0 Diff: https://git.reviewboard.kde.org/r/126212/diff/ Testing --- Now prints an error message instead of crashing. Thanks, Sven Brauch ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman

Re: Review Request 126212: Do not crash if KDE platform integration is loaded but SNI is unavailable

2015-11-30 Thread Sven Brauch
---- On Dec. 1, 2015, 12:53 a.m., Sven Brauch wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126212/ > -

Re: Review Request 126212: Do not crash if KDE platform integration is loaded but SNI is unavailable

2015-11-30 Thread Sven Brauch
is broken, but it's just an env var and it's hard to track down. Diffs - src/kstatusnotifieritem.cpp c48f4d0 Diff: https://git.reviewboard.kde.org/r/126212/diff/ Testing --- Now prints an error message instead of crashing. Thanks, Sven Brauch

Re: Review Request 126212: Do not crash if KDE platform integration is loaded but SNI is unavailable

2015-11-30 Thread Sven Brauch
to this issue is broken, but it's just an env var and it's hard to track down. Diffs (updated) - src/kstatusnotifieritem.cpp c48f4d0 Diff: https://git.reviewboard.kde.org/r/126212/diff/ Testing --- Now prints an error message instead of crashing. Thanks, Sven Brauch

Re: Review Request 126212: Do not crash if KDE platform integration is loaded but SNI is unavailable

2015-11-30 Thread Sven Brauch
is broken, but it's just an env var and it's hard to track down. Diffs - src/kstatusnotifieritem.cpp c48f4d0 Diff: https://git.reviewboard.kde.org/r/126212/diff/ Testing --- Now prints an error message instead of crashing. Thanks, Sven Brauch

Re: Review Request 127122: [Kate] Back/Forward mouse buttons

2016-03-28 Thread Sven Brauch
> On March 28, 2016, 8:22 p.m., Sven Brauch wrote: > > Hmm. Do we really need code for that? Can't you simply assign those buttons > > to the forward / backward actions as shortcuts? Try setting them as > > alternate shortcuts by default. > > Anthony Fieroni wrote

Re: Review Request 127122: [Kate] Back/Forward mouse buttons

2016-03-28 Thread Sven Brauch
assign those buttons to the forward / backward actions as shortcuts? Try setting them as alternate shortcuts by default. - Sven Brauch On March 28, 2016, 8:20 p.m., Anthony Fieroni wrote: > > --- > This is an automatically generat

Re: Review Request 127122: [Kate] Back/Forward mouse buttons

2016-03-28 Thread Sven Brauch
> On March 28, 2016, 8:22 p.m., Sven Brauch wrote: > > Hmm. Do we really need code for that? Can't you simply assign those buttons > > to the forward / backward actions as shortcuts? Try setting them as > > alternate shortcuts by default. > > Anthony Fieroni wrote

Re: gcode.xml got into KF5::KTextEditor without review

2016-03-05 Thread Sven Brauch
Hey Alexander, Thanks for caring and sorry for breaking the string freeze; I missed that. I reverted the commit and I will resubmit it after the 5.8 release is out. On 05/03/16 01:01, Alexander Potashev wrote: > [2] is clearly a feature, but it was not submitted to reviewboard > before pushing

[Differential] [Commented On] D4234: Change algorithm for autobrace.

2017-01-22 Thread Sven Brauch
brauch added a comment. I'm a bit skeptical, this means I can't add closing braces when they are missing ... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: cactus,

[Differential] [Commented On] D4234: Change algorithm for autobrace.

2017-01-23 Thread Sven Brauch
brauch added a comment. Let's discuss an alternative suggestion maybe: how about it just keeps track of the parenthesis balancing and removes them if doing so would make it unbalanced? It could stop counting at the next folding region, and exclude spellchecked parts. REPOSITORY R39

[Differential] [Commented On] D4538: [KTextEditor] consistent conversion from/to cursor to/from coordinates

2017-02-18 Thread Sven Brauch
brauch added a comment. Looks sensible to me, but can you check that it doesn't break KDevelop's navigation widget? I think that is the most visible use case for that interface right now. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4538 EMAIL PREFERENCES

[Differential] [Commented On] D4234: Change algorithm for autobrace.

2017-02-14 Thread Sven Brauch
brauch added a comment. In https://phabricator.kde.org/D4234#86372, @anthonyfieroni wrote: > You mean when we have 3 open when 1 is close to be inserted 2 more to balance counting ? It's not a good idea, about me. No, but if you type a closing one right before a closing one,

[Differential] [Commented On] D4234: Change algorithm for autobrace.

2017-02-12 Thread Sven Brauch
brauch added a comment. Sorry, I wanted to write a reply but failed. My idea was simply to have the algorithm always aim to make parentheses balanced when closing one. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 EMAIL PREFERENCES

Re: Minimum required cmake version

2017-01-03 Thread Sven Brauch
Hey, thanks for the discussion. I've changed the required cmake version to 3.0 for all frameworks, hope I didn't miss any. Best regards, Sven On 27/12/16 17:48, Sven Brauch wrote: > Hi, > > building on CentOS 6.8, I noticed some frameworks (namely KAuth, > KSyntaxHighlighting) do

Re: Minimum required cmake version

2017-01-03 Thread Sven Brauch
Hey, On 03/01/17 14:13, David Faure wrote: > Sven, your commits broke knewstuff and some other modules. > > CMake Error at /d/kde/inst/kde_frameworks/share/ECM/modules/ > ECMGeneratePriFile.cmake:137 (message): > Required variable PROJECT_VERSION_STRING not set before >

Re: Minimum required cmake version

2017-01-03 Thread Sven Brauch
On 03/01/17 14:13, David Faure wrote: >> No objection. > > I do, however, have an objection to untested commits. > > Sven, your commits broke knewstuff and some other modules. > > CMake Error at /d/kde/inst/kde_frameworks/share/ECM/modules/ > ECMGeneratePriFile.cmake:137 (message): > Required

Re: Minimum required cmake version

2016-12-31 Thread Sven Brauch
Hey, On 27/12/16 19:30, Aleix Pol wrote: > +1 on changing to 3.0. Thanks for your opinion on the matter. I also think we should just bump it, 3.0 is old enough -- as said it's even easily available on CentOS 6. Question is, should I change it on _all_ frameworks or just those using Q_GADGET

Re: Minimum required cmake version

2016-12-27 Thread Sven Brauch
Hi, On 27/12/16 19:18, Aleix Pol wrote: > Have you checked that changing to cmake 3.0 actually solves this > problem? I have 3.6 on the same system and with that it works. I don't have 3.0. :/ However, 3.0 docs explicitly mentions Q_GADGET. Greetings, Sven signature.asc Description: OpenPGP

Minimum required cmake version

2016-12-27 Thread Sven Brauch
Hi, building on CentOS 6.8, I noticed some frameworks (namely KAuth, KSyntaxHighlighting) don't actually build with cmake 2.8 -- it seems to be related with moc'ing the Q_GADGET macro. Still, they specify cmake 2.8 as the minimum required cmake version. Do we - attempt to fix this with cmake

Re: Minimum required cmake version

2016-12-27 Thread Sven Brauch
On 27/12/16 19:23, Aleix Pol wrote: > Makes sense, although you still won't be able to compile these > frameworks on CentOS 6.8 even if we fix that, right? I have CMake 3.6 on CentOS 6.8, it's just called "camke3" instead of "cmake". That's why I was running cmake 2.8 so far, and noticed this

D4234: Change algorithm for autobrace.

2017-03-19 Thread Sven Brauch
brauch added a comment. Hm, I think the biggest problem is that we don't know how we want it to work ... so we can't write tests either ;) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 To: cactus, #ktexteditor, mwolff Cc: mwolff, anthonyfieroni, dhaumann,

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

2017-08-03 Thread Sven Brauch
brauch updated this revision to Diff 17638. brauch added a comment. Right. Better call it here. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7010?vs=17411=17638 REVISION DETAIL https://phabricator.kde.org/D7010 AFFECTED FILES src/ksqueezedtextlabel.cpp To: brauch, cfeck Cc:

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

2017-08-10 Thread Sven Brauch
brauch added a comment. I can write a test if you think this helps. I think reading the docs it is quite clear we must call updateGeometry() here: our sizeHint() changes when changing the text. REVISION DETAIL https://phabricator.kde.org/D7010 To: brauch, cfeck, rkflx Cc: dhaumann,

D6870: add missing standard types to C highlighting and update to C11

2017-07-24 Thread Sven Brauch
brauch created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY See bug: https://bugs.kde.org/show_bug.cgi?id=367798 My suggestion is to add the new C11 keywords to the C highlighting, add the

D6870: add missing standard types to C highlighting and update to C11

2017-07-24 Thread Sven Brauch
brauch added reviewers: vkrause, dhaumann. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D6870 To: brauch, vkrause, dhaumann Cc: vkrause, dhaumann, #frameworks

D6870: add missing standard types to C highlighting and update to C11

2017-07-24 Thread Sven Brauch
This revision was automatically updated to reflect the committed changes. Closed by commit R216:c40ad3513f91: add missing standard types to C highlighting and update to C11 (authored by brauch). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6870?vs=17104=17109#toc REPOSITORY R216

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

2017-07-30 Thread Sven Brauch
brauch added a comment. With enough dedication, probably yes ... REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D7010 To: brauch, cfeck Cc: aacid, #frameworks

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

2017-07-30 Thread Sven Brauch
brauch created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY By changing the full text, the sizeHint() of the widget changes. If the

D8333: fix some indenters from randomly invoking indent

2017-10-16 Thread Sven Brauch
brauch created this revision. brauch added reviewers: KTextEditor, cullmann, dhaumann. Restricted Application added projects: Kate, Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY If triggerCharacters was not set, toString() would return "undefined", making

D8333: fix some indenters from randomly invoking indent

2017-10-16 Thread Sven Brauch
brauch marked an inline comment as done. brauch added inline comments. INLINE COMMENTS > anthonyfieroni wrote in kateindentscript.cpp:47 > But if triggerCharacters are undefined this variable should be false, no? No, that's still fine, this variable just caches reading the value from the

D8333: fix some indenters from randomly invoking indent

2017-10-16 Thread Sven Brauch
brauch marked 2 inline comments as done. brauch added a comment. Yes, at least the ones I'm aware of. Thanks for the review, I'll push it in a moment. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D8333 To: brauch, #ktexteditor, cullmann, dhaumann Cc:

D8333: fix some indenters from randomly invoking indent

2017-10-16 Thread Sven Brauch
This revision was automatically updated to reflect the committed changes. Closed by commit R39:aeebeadb5f59: fix some indenters from indenting on random characters (authored by brauch). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8333?vs=20861=20874

D12662: Add InlineNoteInterface

2018-05-07 Thread Sven Brauch
brauch added inline comments. INLINE COMMENTS > dhaumann wrote in katedocument.cpp:5316-5320 > Btw, may I am wrong here, since this is really called often when painting > lines... So this may be good to have. I also stumbled upon it but it avoids an alloc in a very deep loop, so it might be

D12768: Allow wrapping selection off top/bottom of autocomplete results

2018-05-09 Thread Sven Brauch
brauch added a comment. What happens if the overload selection window is open in addition (like in KDevelop)? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12768 To: sraizada, #ktexteditor Cc: brauch, #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham,

D13365: Fixed the cursor(caret) width in kate

2018-06-06 Thread Sven Brauch
brauch added a comment. Click "download raw diff" at the top-right of the page. This is the diff you uploaded. This is not the change we want to apply when merging this review request; it only contains the changes you did from your earlier version to the latest one, not the set of changes

D13365: Fixed the cursor(caret) width in kate

2018-06-06 Thread Sven Brauch
brauch added a comment. Yes that looks better, I'll submit it later today. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13365 To: shubham, #ktexteditor, brauch, cullmann Cc: cullmann, ngraham, brauch, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew,

D13442: Implemented displaying of total lines in kate

2018-06-13 Thread Sven Brauch
brauch added a comment. I was also thinking about the context menu, yes. That is also where a lot of users would first look for it I think, even before the config dialog. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To: shubham, #ktexteditor, cullmann,

D13442: Implemented displaying of total lines in kate

2018-06-13 Thread Sven Brauch
brauch added a comment. I don't like this being user-controllable. We spend so much effort in keeping our option set clearly arranged and limited to useful choices, to me this seems like a very bad opportunity to break that rule. REPOSITORY R39 KTextEditor REVISION DETAIL

D13442: Implemented displaying of total lines in kate

2018-06-09 Thread Sven Brauch
brauch added a comment. Since the other two number displays use QLocale().toString(), probaly this should too ... REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13442 To: shubham, #ktexteditor, cullmann, brauch Cc: kwrite-devel, kde-frameworks-devel, michaelh,

D13365: BUG:391518 Fixed the cursor(caret) width in kate

2018-06-05 Thread Sven Brauch
brauch added a comment. Good find (annoyed me too) and sounds plausible to me. Thanks! Just one thing, do you call setRenderHint() inside the save() / restore() pair of the painter so the previous state is restored after drawing the caret? REPOSITORY R39 KTextEditor REVISION DETAIL

D13365: BUG:391518 Fixed the cursor(caret) width in kate

2018-06-05 Thread Sven Brauch
brauch added a comment. Good find (annoyed me too) and sounds plausible to me. Thanks! Just one thing, do you call setRenderHint() inside the save() / restore() pair of the painter so the previous state is restored after drawing the caret? REPOSITORY R39 KTextEditor REVISION DETAIL

D13365: Fixed the cursor(caret) width in kate

2018-06-05 Thread Sven Brauch
brauch accepted this revision. brauch added a comment. This revision is now accepted and ready to land. Yep, with that, it looks good to me. Do you have commit access? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13365 To: shubham, #ktexteditor, brauch Cc:

D13365: Fixed the cursor(caret) width in kate

2018-06-06 Thread Sven Brauch
brauch added a comment. The change is good now but you screwed up the patch: you need to submit the full set of changes you want to do as a patch, not only the last iteration. Can you fix that? Thank you! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13365

D13365: Fixed the cursor(caret) width in kate

2018-06-07 Thread Sven Brauch
brauch added a comment. I'm sorry I am so annoying, but iirc KDE's commit hooks will not accept commits with only one name fragment. Something equivalent to the western first/last name pair is required. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13365 To:

D13365: Fixed the cursor(caret) width in kate

2018-06-07 Thread Sven Brauch
This revision was automatically updated to reflect the committed changes. Closed by commit R39:e6f87dd57008: Fix caret width (authored by shubham, committed by brauch). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13365?vs=35683=35735 REVISION DETAIL

D13365: Fixed the cursor(caret) width in kate

2018-06-06 Thread Sven Brauch
brauch added a comment. To submit the change with your name on it, I'd need a full name and email address, can you provide that? Thanks! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D13365 To: shubham, #ktexteditor, brauch, cullmann Cc: cullmann, ngraham,

D12662: Add InlineNoteInterface

2018-05-02 Thread Sven Brauch
brauch added a comment. Awesome idea! Do you have a screenshot of how it looks? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12662 To: michalsrb, #ktexteditor Cc: brauch, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann

D12662: Add InlineNoteInterface

2018-05-02 Thread Sven Brauch
brauch added a comment. Looks good from the implementation too so far. One thing I do not see is any changes to the cursorToX / xToCursor functions, is there really no change required there? Some things which come to my mind for testing would be: - is selection rendered correctly

D12662: Add InlineNoteInterface

2018-05-03 Thread Sven Brauch
brauch added a comment. I think fixing the selection rendering issue would be nice. Regarding blockmode behaviour, I think it will be a bit strange either way. One thing to compare is the behaviour with non-fixed width fonts: there block mode also selects constant columns (i.e. it will

D12662: Add InlineNoteInterface

2018-05-03 Thread Sven Brauch
brauch added a comment. Oh, heh, yes it does. It's just your docstring which says otherwise ("is an interface for the View"), and that's what I looked at at that time. That should be changed. ;) I think the question is less what you need and more what it is supposed to do semantically.

D9569: Fix wildcard matching for modelines

2017-12-30 Thread Sven Brauch
brauch accepted this revision. brauch added a comment. This revision is now accepted and ready to land. Heh. Fix looks obviously correct to me (good find), and tests are always nice. REPOSITORY R39 KTextEditor BRANCH TestModelines (branched from master) REVISION DETAIL

D7660: Fix a regression caused by changing backspace key behavior

2017-12-23 Thread Sven Brauch
brauch reopened this revision. brauch added inline comments. INLINE COMMENTS > katedocument_test.cpp:452 > auto view = > static_cast(doc.createView(nullptr)); > +view.config()->setBackspaceRemoveComposed(true); >

D7660: Fix a regression caused by changing backspace key behavior

2017-12-23 Thread Sven Brauch
brauch added a comment. After this was submitted master doesn't compile for me, and if I fix the compile in the trivial way the test fails. Can you have another look? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D7660 To: safaalfulaij, #ktexteditor, jgrulich,

D14826: inline note interface wip #2

2018-08-15 Thread Sven Brauch
brauch updated this revision to Diff 39815. brauch added a comment. address Dominik's suggestion and split focus handling and click handling REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14826?vs=39802=39815 BRANCH inlinenotes REVISION DETAIL

D14826: inline note interface wip #2

2018-08-15 Thread Sven Brauch
brauch updated this revision to Diff 39802. brauch added a comment. I added the rest of the interaction interface (click, mouseover) and reduced the API a bit by moving a few hints into the InlineNote object. Only thing I still intend to change about the API would be that we use the

D14826: inline note interface wip #2

2018-08-14 Thread Sven Brauch
brauch created this revision. Herald added projects: Kate, Frameworks. Herald added subscribers: kde-frameworks-devel, kwrite-devel. brauch requested review of this revision. REPOSITORY R39 KTextEditor BRANCH master REVISION DETAIL https://phabricator.kde.org/D14826 AFFECTED FILES

D14826: inline note interface wip #2

2018-08-14 Thread Sven Brauch
brauch added reviewers: michalsrb, dhaumann, cullmann. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14826 To: brauch, michalsrb, dhaumann, cullmann Cc: kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars, dhaumann

D5802: ViewPrivate, KateSearchBar, KateVi::MatchHighlighter: use selection foreground for search highlights

2018-08-14 Thread Sven Brauch
brauch added a comment. Herald added a project: Kate. Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks. Can't we simply update our shipped schemas, and expect users with custom schemas to fix them? REPOSITORY R39 KTextEditor REVISION DETAIL

D14826: inline note interface wip #2

2018-08-16 Thread Sven Brauch
brauch updated this revision to Diff 39889. brauch added a comment. update license text REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14826?vs=39888=39889 BRANCH inlinenotes REVISION DETAIL https://phabricator.kde.org/D14826 AFFECTED FILES

D14826: inline note interface wip #2

2018-08-16 Thread Sven Brauch
brauch updated this revision to Diff 39888. brauch marked 21 inline comments as done. brauch added a comment. Implement Dominik's suggestions REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14826?vs=39815=39888 BRANCH inlinenotes REVISION DETAIL

D14826: inline note interface wip #2

2018-08-16 Thread Sven Brauch
brauch closed this revision. brauch added a comment. commit 4ea5fee0afe5c76bbee07563c23ede808aa059de Author: Sven Brauch Date: Tue Aug 14 12:31:31 2018 +0200 Add inline note interface Original patch by Michal Srb. The inline note interface provides a way

D14894: [RFC] Fix block mode for multi-cursor branch

2018-08-17 Thread Sven Brauch
brauch added a comment. Thanks for the patch, the approach looks reasonable at a first glance. You might want to unite the up/down functions ... It is too much of a WiP to merge it like this, though. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14894 To:

D14897: InlineNote: Pimpl inline note data without allocs

2018-08-17 Thread Sven Brauch
brauch added a comment. Looks ok to me, except one thing: the operator== is used to compare a note from the list to the "currently active" note in the view. If this compares also the "under mouse" state, this code might be broken now ...? REPOSITORY R39 KTextEditor REVISION DETAIL

D14897: InlineNote: Pimpl inline note data without allocs

2018-08-17 Thread Sven Brauch
brauch added a comment. Sorry, never mind -- that code I removed yesterday. All should be fine. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14897 To: dhaumann, cullmann Cc: brauch, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns,

D12662: Add InlineNoteInterface

2018-08-12 Thread Sven Brauch
brauch added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. Hey there, what's happening to this? I think this is a really nice feature and it would be very sad if it would bitrot :( Anything anyone can assist with? We

D14773: completion widget: fix minimum header section size

2018-08-12 Thread Sven Brauch
brauch created this revision. brauch added a reviewer: Kate. Restricted Application added projects: Kate, Frameworks. Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. brauch requested review of this revision. REVISION SUMMARY The header is hidden, but if the minimum

D14758: use view lines for wheel scrolling, not real lines

2018-08-12 Thread Sven Brauch
brauch added a comment. I will modify the commit message as suggested. I read through the bug report, and I think this patch does fix the main issue discussed there. The patch suggested by one user there is even quite similar (if a bit less complete) than this one. The original

D14758: use view lines for wheel scrolling, not real lines

2018-08-12 Thread Sven Brauch
brauch added a comment. The "overly sensitive touchpad" issue seems to be missing accumulation of scroll events, so this patch to my understanding should not have it. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14758 To: brauch, dhaumann Cc: ngraham,

D14758: use view lines for wheel scrolling, not real lines

2018-08-12 Thread Sven Brauch
brauch created this revision. brauch added a reviewer: dhaumann. Restricted Application added projects: Kate, Frameworks. Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. brauch requested review of this revision. REVISION SUMMARY I don't think there is an ideal

D12662: Add InlineNoteInterface

2018-08-13 Thread Sven Brauch
brauch added a comment. Wow, that looks amazing! Really impressive. Using moving cursors for this sounds good to me. Other things which cannot be updated in real-time do that as well (highlighting, error underlines, ...) Regarding the cursor issue: I think you would need a state

D12662: Add InlineNoteInterface

2018-08-13 Thread Sven Brauch
brauch added a comment. By the way, other people around here are very impressed by this patch as well, and we'd really like to get this merged :) Moving e.g. KDevelop's warning markers into an inline note instead of the annoying popup would make a real difference for usability ...

D14773: completion widget: fix minimum header section size

2018-08-13 Thread Sven Brauch
This revision was automatically updated to reflect the committed changes. Closed by commit R39:5499a0df825c: completion widget: fix minimum header section size (authored by brauch). REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14773?vs=39533=39562

D14758: use view lines for wheel scrolling, not real lines

2018-08-13 Thread Sven Brauch
This revision was automatically updated to reflect the committed changes. Closed by commit R39:2815fea7854f: Fix: Scroll view lines instead of real lines for wheel and touchpad scrolling (authored by brauch). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D14758?vs=39499=39561#toc

D14758: use view lines for wheel scrolling, not real lines

2018-08-13 Thread Sven Brauch
brauch added a comment. In D14758#307401 , @ngraham wrote: > In D14758#307333 , @brauch wrote: > > > The "overly sensitive touchpad" issue seems to be missing accumulation of scroll events, so this

D12662: Add InlineNoteInterface

2018-08-13 Thread Sven Brauch
brauch added a comment. I'd like to play with this a bit wrt what can be done in KDevelop with it (I want the problem popups gone). Would you mind if I do some changes along the way? I would post an updated patch here, in case I actually come up with useful changes ... REPOSITORY R39

D14826: inline note interface wip #2

2018-08-14 Thread Sven Brauch
brauch added a comment. Thanks for the feedback! I will try doing a few more things with this interace and then maybe discuss again with the other kate people here at Akademy about which one they like better. About the tracking, I don't think anything is needed on the side of the

D14826: inline note interface wip #2

2018-08-14 Thread Sven Brauch
brauch added a comment. Sample patch for KDevelop's problem highlighter plus screenshot: F6192637: hl.png F6192639: inline-problems.diff REPOSITORY R39 KTextEditor REVISION DETAIL

D14826: inline note interface wip #2

2018-08-14 Thread Sven Brauch
brauch updated this revision to Diff 39692. brauch added a comment. add missing files REPOSITORY R39 KTextEditor CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14826?vs=39682=39692 BRANCH inlinenotes REVISION DETAIL https://phabricator.kde.org/D14826 AFFECTED FILES

D14826: inline note interface wip #2

2018-08-14 Thread Sven Brauch
brauch edited the summary of this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D14826 To: brauch, michalsrb, dhaumann, cullmann Cc: anthonyfieroni, kwrite-devel, kde-frameworks-devel, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, sars,

D14826: inline note interface wip #2

2018-08-14 Thread Sven Brauch
brauch updated this revision to Diff 39721. brauch added a comment. add noteActivated notifier function When a note is mouse-overed or clicked, a function on the note provider is called, giving the point it was hovered or clicked in note coordinates, the type of event, and the note

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

2018-09-05 Thread Sven Brauch
brauch added a comment. For the record, I tried writing a test for this but didn't succeed and eventually put it aside, although the difference is easily visible in a test application. There must be a reason why the naive test case behaves differently from an interactive application ... I

D12016: [ktexteditor] much faster positionFromCursor

2018-04-09 Thread Sven Brauch
brauch added a comment. Oh and, you do not need to inherit QObject to use connect; you can connect to a lambda calling the member function AFAIK or so. Just omit the third argument in connect(). What you lose by doing this is the automatic disconnect of the connection when the receiver

D12016: [ktexteditor] much faster positionFromCursor

2018-04-09 Thread Sven Brauch
brauch added a comment. Re. binary compatibility: should be fine because this class is not exported (no KTEXTEDITOR_EXPORT macro). REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D12016 To: jtamate, #kate, cullmann, #frameworks Cc: brauch, cullmann, #frameworks,

D11811: avoid Asan runtime error: shift exponent -1 is negative

2018-03-30 Thread Sven Brauch
brauch added a comment. Change looks good (the previous code definitely looks like nonsense), but what does this mean for existing settings, saved previously? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11811 To: jtamate, #kate, #frameworks Cc: brauch,

D11838: Turn on line numbers by default

2018-04-01 Thread Sven Brauch
brauch added a comment. For KDevelop this is fine, I don't think we have any objection. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D11838 To: ngraham, #kate, #ktexteditor, dhaumann, mludwig Cc: brauch, mludwig, kfunk, dhaumann, #frameworks, michaelh,

D17459: SearchBar: Add Cancel button to stop long running tasks

2018-12-10 Thread Sven Brauch
brauch added a comment. I would also advise against calling processEvents() to keep UIs responsive in all cases. It's tempting, but it is near impossible to get it right. What about conflicting actions, close/resize events, dbus calls, etc etc that are handled here? I think the right

D17241: Disable highlighting for lines longer than 1024 characters.

2018-11-29 Thread Sven Brauch
brauch added a comment. Shouldn't this change simultaneously remove the line length limit ...? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D17241 To: sars, cullmann, vkrause Cc: brauch, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns,

D19764: Fix Minimap with QtCurve style

2019-03-29 Thread Sven Brauch
brauch accepted this revision. brauch added a comment. This revision is now accepted and ready to land. Looks sensible, and I think I've even seen this bug already somewhere. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19764 To: sars, brauch, dhaumann,

D19621: ViewPrivate: Make deselection by arrow keys more handy

2019-03-09 Thread Sven Brauch
brauch added a comment. Hm, so shift-selecting with Shift+Left 5 times, then pressing Shift+Right once clears the selection? Just for the statistics, deselecting one character is a feature I use all the time. Why is this behaviour helpful? If you want to clear the selection, press Esc.

  1   2   >