Fixing KTextEditor Autobraces

[Subramaniyam Raizada]

Hi,

KTextEditor has bad behaviour with autobraces. With autobraces enabled,
typing the following will work:
if(abc)
The user will type "if(", the closing ")" will automatically be
generated, and when the user types ")", the auto-generated ")" will be
'eaten.'

However, nesting braces or quotes at any level fails. If the user types
if(abc or (def))
The first ) in "def)" will be eaten, but the final ) will not be eaten.
So by typing two open parens and two close parens, the result will be
if(abc or (def))). This is the same for any other type of open/close -
braces, brackets, strings (e.g. if("abc") will become if("abc")) since
the end " will get eaten but then the ) won't get eaten).

I have mostly fixed this behaviour, but would appreciate some help in
fixing out one last bug. In the file
ktexteditor/src/document/katedocument.cpp (and the header file), I
changed QChar m_currentAutobraceClosingChar to a QStack<QChar>. Upon
entering a ([{"', it gets pushed onto the stack, and putting a closing
one will eat the input and pop it, if the user entered the correct
closing character.

There's one (I think easy) problem left, but I don't know the
codebase/C++ well enough to figure it out. Moving the cursor or pressing
the left/right keys should clear the stack (this happens both with the
current autobrace behaviour, and in other IDEs that support nested
autobraces).

The current code uses m_currentAutobraceClosing char to remember what
type of character ({["' was opened, and m_currentAutobraceRange to check
whether the cursor is in a place where an end brace should be eaten.
Upon moving the cursor or closing the brace, currentAutobraceRange gets
reset/cleared. However, now that autobraces can nest, the range should
reset only upon moving the cursor or closing the last brace on the
stack, not on closing any brace.

I have attached the modified katedocument.cpp file with my current
changes and some qPrint statements to see what's going on. There are
three places where the autobraceRange gets reset.

The first is at line 2951 of the attached file; it is called upon
entering any end bracket. I wrapped that in an if statement to only
trigger if the stack is emptied.

The second is on line 3057. It does trigger sometimes, but doesn't seem
to interfere with the desired behaviour.

The final time autobraceRange is reset or cleared is on line 3090, in
the checkCursorForAutobrace function (which is called right after the
clear on line 3057). From watching the output in the terminal, this
function clears the autobrace A) upon moving the cursor and B) upon
closing any pair of braces. It's fine to clear the autobrace range upon
moving the cursor, but it shouldn't clear the autobrace range upon
ending a brace pair, unless that's the final pair on the stack.

The function does:
if (m_currentAutobraceRange &&
....!m_currentAutobraceRange->toRange().contains(newPos)) {
        qCritical() << "autobrace range reset from 3";
        m_currentAutobraceRange.clear();
}

So it seems to do 'if the cursor has moved out of the autobrace range,
clear the range.' The solution, then, is to get the autobrace range to
hold all open braces, instead of just going up to one end brace.

The second autobraceReset, on line 3057, doesn't clear the autobrace
range, but sets it to some actual range of text. Its code is this:
m_currentAutobraceRange.reset(
    newMovingRange({cursorPos - Cursor{0, 1},
                      insertedAt},
                   KTextEditor::MovingRange::DoNotExpand));

Upon inserting an open brace, this apparently sets the autobraceRange
from the cursor position up to the end of the inserted end-brace. It
should, however, set the range from the cursor position up to the end of
(inserted end-brace + number of braces on the stack
m_currentAutobraceClosingChar). That's not a perfect solution, but
should work in 99% of cases.

However, the first line of the previous paragraph (sets it to cursor pos
+ end of current brace) is just pure conjecture, because this is where
my lack of C[++] knowledge becomes an issue. I'm experienced in Java and
Python, and was able to get the QChar converted to a QStack<QChar> and
push/pop from that correctly (m_currentAutobraceClosingCharacter should
also be renamed to reflect this, I suppose), but I have no idea what the
{cursorPos - Cursor{0, 1}, insertedAt} in the newMovingRange function does.

I was able to search up newMovingRange definition on the github mirror -
it's on line 83 of src/include/ktexteditor/movinginterface.h, but trying
to find the constructor for Cursor() gave >800 results. So I would
really appreciate it if someone could help me get the
{cursorPos - Cursor{0, 1}, insertedAt}
to include all the braces on the stack, instead of just the one
auto-inserted end brace.

Also, in case you want to test the current behaviour, commenting out the
m_currentAutobraceRange.clear() on line 3090 will get the nested quotes
behaviour fully working, with the exception of not resetting the stack
when uneaten quotes are left (so it'll work fine as long as you don't
move the cursor until manually entering all '"]}) remaining). A short
video of this is attached.

Thanks,
Sub Raizada
-------------- next part --------------
A non-text attachment was scrubbed...
Name: katedocument.cpp
Type: text/x-c++src
Size: 191399 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20180417/a1c84213/attachment-0001.cpp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: recording.mp4
Type: video/mp4
Size: 53331 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20180417/a1c84213/attachment-0001.mp4>