D4234: Change algorithm for autobrace.

cullmann abandoned this revision. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 To: cullmann, #ktexteditor, mwolff, brauch, cactus Cc: kde-frameworks-devel, nalvarez, mwolff, anthonyfieroni, dhaumann, brauch, cullmann, kwrite-devel, domson, michaelh, ngraham,

D4234: Change algorithm for autobrace.

cullmann commandeered this revision. cullmann added a reviewer: cactus. cullmann added a comment. We altered stuff in D19608 . Could you try if that fits your needs? If not, perhaps we need a new review for what can be further improved, thanks. REPOSITOR

D4234: Change algorithm for autobrace.

cullmann added a reviewer: brauch. Herald added a project: Kate. Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 To: cactus, #ktexteditor, mwolff, brauch Cc: kde-frameworks-devel, nalvar

D4234: Change algorithm for autobrace.

nalvarez added a comment. Writing tests for this might not be easy, so even having passing tests for the current behavior would be very useful, as a first step. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 To: cactus, #ktexteditor, mwolff Cc: nalvarez, mw

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,

D4234: Change algorithm for autobrace.

mwolff requested changes to this revision. mwolff added a comment. This revision now requires changes to proceed. I'm personally all for improving the status quo, but I think the biggest problem here is that we have no unit test coverage (or do we?). The unit tests would also clearly show the

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

dhaumann added a comment. Ok, I see. Then I would like you to look at how Qt Creator behaves here. And agree, then the proposed patch is maybe better than thought. If you want, you can even look into the implementation of Qt Creator... REPOSITORY R39 KTextEditor REVISION DETAIL https://

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

brauch added a comment. Hm ok, I never hit that problem, I guess we're just different there. I have muscle memory which types f() or f(x) or "hi", but not for more than one level of closing parentheses. I'm really sceptical of the "always eat parentheses" suggestion, it seems too extrem

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

cactus added a comment. Hello, In the case of void foo(bar); the both solution has the same result. But when you have 2 levels of bracket the result are different. In the example of : if (isEndMatching()) or printf("Bonjour") With auto-brackets enabled and with the old behav

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

dhaumann added a comment. In my opinion, the current solution is "ok", i.e., if I type: void foo(bar); with auto-brackets enabled, the current code is smart enough eat the ')' since it maintains the range. While defining new functions or calling methods, this works perfectly. The propos

[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, "eat"

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

anthonyfieroni added a comment. In https://phabricator.kde.org/D4234#85804, @brauch wrote: > 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. You mean when we have 3 open when 1 is close

[Differential] [Changed Subscribers] D4234: Change algorithm for autobrace.

anthonyfieroni added inline comments. INLINE COMMENTS > katedocument.cpp:2920-2921 > +(isEndBracket(chars[0]) || > + chars[0] == QLatin1Char('\'') || > + chars[0] == QLatin1Char('\"'))) { > + Why seach a checks ? REPOSITORY R39 KTextEditor RE

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

cullmann added a comment. As I don't use this myself, my opinion won't really matter. Sven, is the new proposed behavior really worse than the old? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 EMAIL PREFERENCES https://phabricator.kde.org/settings/pane

[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 https://phabricator.kd

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

dhaumann added a comment. What's the current state of this? REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: cactus, #ktexteditor Cc: dhaumann, brauch, cullmann, kwrite-devel

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

cactus added a comment. Sorry, I am not sure to understand what do you want. You would like the previous behavior with a multi-depth stack of range? I don't understand what disturb you in my solution. You can close a missing bracket. If you have (| and you press ')', you obtain ()|. Bu

[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 KTextE

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

cactus added a comment. In https://phabricator.kde.org/D4234#79253, @brauch wrote: > I'm a bit skeptical, this means I can't add closing braces when they are missing ... Yes, I agree with you. If you are in this situation: (((|)) with | for the cursor, you must type 3 ')' to clos

[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, #ktexte

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

cullmann added a comment. Given it eliminates some members, this looks nice, thought I don't use that feature enough to judge if the behavior change is wanted. REPOSITORY R39 KTextEditor REVISION DETAIL https://phabricator.kde.org/D4234 EMAIL PREFERENCES https://phabricator.kde.org/se

[Differential] [Request, 68 lines] D4234: Change algorithm for autobrace.

cactus created this revision. cactus added a reviewer: KTextEditor. Restricted Application added subscribers: Frameworks, kwrite-devel. Restricted Application added a project: Frameworks. REVISION SUMMARY The autobrace ignore closing brace when it already is here. REPOSITORY R39 KTextEditor