D29789: Make text always align with font base line

brauch added a comment. Hmm, consider though that a configuration option should be something that gives a choice to the user. It shouldn't be necessary to set a config option in order to make the program behave correctly. REPOSITORY R39 KTextEditor REVISION DETAIL

D25339: KateRenderer: Use representitive character in CJK to estimate the fontHeight.

brauch added a comment. In D25339#663915 , @fakefred wrote: > I second the as-an-option proposal. Hey, why not automatically increase the line height when CJK characters are detected? This issue has been around for years and actually

D27912: Draw inlineNotes after drawing word wrap marker

brauch added a comment. I also don't understand this. Even if the painting somehow changes, e.g. because some painter state is set which wasnt set before (which I do not see to be the case here), that should not affect the line layout, as that is computed separately. Very strange.

D27912: Draw inlineNotes after drawing word wrap marker

brauch accepted this revision. brauch added a comment. This revision is now accepted and ready to land. Looks good, thanks! Yes, it's an improvement. REPOSITORY R39 KTextEditor BRANCH caret REVISION DETAIL https://phabricator.kde.org/D27912 To: davidre, #ktexteditor, brauch Cc:

D26840: Correctly set underMouse() for inline notes

This revision was automatically updated to reflect the committed changes. Closed by commit R39:7f043fbb26d4: inline notes: correctly set underMouse() for inline notes (authored by brauch). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D26840?vs=74110=74351#toc REPOSITORY R39

D26840: Correctly set underMouse() for inline notes

brauch added a comment. Ok, done ;) REPOSITORY R39 KTextEditor BRANCH underMouse (branched from master) REVISION DETAIL https://phabricator.kde.org/D26840 To: davidre, #ktexteditor, cullmann, brauch Cc: brauch, cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, GB_2,

D26840: Correctly set underMouse() for inline notes

brauch accepted this revision. brauch added a comment. If you want I can integrate these changes and submit your patch, should I? Thanks a lot for your contribution. REPOSITORY R39 KTextEditor BRANCH underMouse (branched from master) REVISION DETAIL https://phabricator.kde.org/D26840

D26840: Correctly set underMouse() for inline notes

brauch added a comment. I'm sorry, updateView is the wrong function to call, you need updateDirty. I tried it out, like this it works: if (e->buttons() == Qt::NoButton) { auto noteData = inlineNoteAt(e->globalPos()); if (noteData.m_position.isValid()) { if

D26840: Correctly set underMouse() for inline notes

brauch added a comment. Then it seems like the line is not tagged correctly. Maybe try `tagLines(note.position.line(), note.position.line())`? I think the column being the same is not what the function being called expects. REPOSITORY R39 KTextEditor BRANCH underMouse (branched from

D26840: Correctly set underMouse() for inline notes

brauch added a comment. My guess is right now the view updates when the cursor blinks, so that's why it updates after a short moment (of varying length, though, if you look at the video). Since the line is tagged dirty, it gets repainted correctly, but too late. REPOSITORY R39

D26840: Correctly set underMouse() for inline notes

brauch added a comment. Hm, yeah, looking at the code I think you might need to call `updateView()` in case a focus in or out happened. Can you try if that makes a difference? REPOSITORY R39 KTextEditor BRANCH underMouse (branched from master) REVISION DETAIL

D26840: Correctly set underMouse() for inline notes

brauch added a comment. Is the video the new behaviour? It still looks a bit weird to me, there is a slight delay between the mouse entering the area and the highlight changing. Is it possible that the line (and thus the note) is not properly marked for repaint when the underMouse

D22511: Minimap: Do not grab left mouse click over up/down arrows

brauch accepted this revision. brauch added a comment. This revision is now accepted and ready to land. LGTM, thank you! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D22511 To: sars, brauch Cc: kwrite-devel, kde-frameworks-devel, LeGast00n, sbergeron, domson,

D22477: With auto completion don't show completions that don't match from beginning of typed word

brauch added a comment. Also here, looks good to me, but I would wait for feedback from somebody else in addition. Thank you for working on this! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D22477 To: ahmadsamir, #ktexteditor, cullmann, dhaumann, #kdevelop,

D22477: With auto completion don't show completions that don't match from beginning of typed word

brauch added inline comments. INLINE COMMENTS > brauch wrote in katecompletionmodel.cpp:2029 > Maybe you want to set the flag here too? Actually no, probably not. ;) REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D22477 To: ahmadsamir, #ktexteditor, cullmann,

D22477: With auto completion don't show completions that don't match from beginning of typed word

brauch added inline comments. INLINE COMMENTS > katecompletionmodel.cpp:2029 > if (matchesAbbreviation(m_nameColumn, match, > model->matchCaseSensitivity())) { > matchCompletion = AbbreviationMatch; > } Maybe you want to set the flag here too? REPOSITORY R39

D22500: Make keyword completion model return HideListIfAutomaticInvocation by default

brauch added a comment. Independent of anything else I think this is a very sensible change, and seems like an oversight / bug. One of the more core kate guys should approve, but +1 from me. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D22500 To:

D22477: Add a config to only show completions matching word beginning

brauch added a comment. I'm not sure if this solves the right problem. Where I notice this issue a lot is when typing "return". The keyword completion suggests "return", and when I want a newline, I complete the "return" first. This doesn't affect C++ (because of the trailing ";"), but

D20852: Fix to show folding preview when move the mouse from bottom to top

brauch added a comment. I guess the intention of the highlight delay is that when you move your mouse across the border, the view doesn't flicker. The 150ms does this well enough for me, I never see a flicker at least ;) REPOSITORY R39 KTextEditor REVISION DETAIL

D19764: Fix Minimap with QtCurve style

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

brauch added a comment. Sorry, yeah, I misunderstood! REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D19621 To: loh.tar, #ktexteditor Cc: cullmann, brauch, dhaumann, ngraham, kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, domson, michaelh, bruns,

D19621: ViewPrivate: Make deselection by arrow keys more handy

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.

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

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.

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,

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

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

D14897: InlineNote: Pimpl inline note data without allocs

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,

D14897: InlineNote: Pimpl inline note data without allocs

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

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

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:

D14826: inline note interface wip #2

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

D14826: inline note interface wip #2

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

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

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

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

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

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

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

D14826: inline note interface wip #2

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

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

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

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

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

D14826: inline note interface wip #2

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

D12662: Add InlineNoteInterface

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

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

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

D14773: completion widget: fix minimum header section size

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

D12662: Add InlineNoteInterface

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 ...

D12662: Add InlineNoteInterface

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

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

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

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

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

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

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

D14773: completion widget: fix minimum header section size

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

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

D13442: Implemented displaying of total lines in kate

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

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

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: Fixed the cursor(caret) width in kate

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

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

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,

D13365: Fixed the cursor(caret) width in kate

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,

D13365: Fixed the cursor(caret) width in kate

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

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

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: BUG:391518 Fixed the cursor(caret) width in kate

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

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

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

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,

D12662: Add InlineNoteInterface

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

D12662: Add InlineNoteInterface

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.

D12662: Add InlineNoteInterface

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

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

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

D12016: [ktexteditor] much faster positionFromCursor

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

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,

D11838: Turn on line numbers by default

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,

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

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,

D9569: Fix wildcard matching for modelines

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

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

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,

D8333: fix some indenters from randomly invoking indent

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

D8333: fix some indenters from randomly invoking indent

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

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

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

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

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,

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

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

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

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

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

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

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

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

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

D4234: Change algorithm for autobrace.

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,

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

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.

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.

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

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

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] D4234: Change algorithm for autobrace.

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,

Re: Minimum required cmake version

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

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

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

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

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

Re: Minimum required cmake version

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

  1   2   >