DocumentPrivate: Make bracket handling smart
ClosedPublic

Authored by loh.tar on Mar 8 2019, 10:16 AM.

Details

Summary

This patch checks if the entered closing bracket is already balanced and
skip the input in this case. Targets the "nested autbracket" problem
without to base on this enabled config setting.

BUG: 368580
FIXED-IN: 5.56

Test Plan
  • Will substitude D12295
  • Perhaps could D19598 also catched here when findMatchingBracket will be modified?

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
loh.tar created this revision.Mar 8 2019, 10:16 AM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptMar 8 2019, 10:16 AM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Mar 8 2019, 10:16 AM
loh.tar updated this revision to Diff 53444.Mar 8 2019, 2:40 PM
loh.tar retitled this revision from [WIP] DocumentPrivate: Make bracket handling smart to DocumentPrivate: Make bracket handling smart.
loh.tar edited the test plan for this revision. (Show Details)
  • Fixes to pass autotests
loh.tar edited the summary of this revision. (Show Details)Mar 8 2019, 4:41 PM
loh.tar set the repository for this revision to R39 KTextEditor.
cullmann accepted this revision.Apr 12 2019, 7:56 PM

Works for me, beside that the patch no longer cleanly applies.
But with some false removed and the later parts skipped, it did work as advertised.

This revision is now accepted and ready to land.Apr 12 2019, 7:56 PM
This revision was automatically updated to reflect the committed changes.
meven added a subscriber: meven.Apr 13 2019, 6:52 AM

I could have made the commit, but heh.

I think this change introduces a regression: This code does not check for "view->config()->autoBrackets()", so it's *ALWAYS* active, which I do not want. Consider this:

if (bla |) { ... }

Now you type && foo(), the result is then:

if (bla && foo() { ... }

Notice the unbalanced closing ), which is due to this patch.

Can we either revert this, or at least add config check?

cullmann reopened this revision.May 19 2019, 11:40 AM

I think moving the config check some lines up will do the trick.

This revision is now accepted and ready to land.May 19 2019, 11:40 AM
This revision was automatically updated to reflect the committed changes.