D12271: Don't remove trailing whitespace from cursor line

[Subramaniyam Raizada]

sraizada created this revision.
sraizada added a reviewer: KTextEditor.
sraizada added a project: KTextEditor.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.
sraizada requested review of this revision.

REVISION SUMMARY
  When the 'remove trailing whitespace' option is enabled, if and only if the 
cursor is at the end of a line with trailing whitespace, the cursor position 
will be held instead of deleting the whitespace its position depends upon. 
Other lines will still have trailing whitespace removed. Useful to not lose 
auto-indent for those who save often.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12271

AFFECTED FILES
  src/document/katedocument.cpp

To: sraizada, #ktexteditor
Cc: #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
head7, cullmann, kfunk, sars, dhaumann

D12271: Don't remove trailing whitespace from cursor line

[Subramaniyam Raizada]

sraizada added a comment.


  Forgot to put this in the first submission: it fails test 66 vimode_keys, I 
believe this may be related to bug 392858 - vi mode :q (quit) command works in 
Kate but not KWrite https://bugs.kde.org/show_bug.cgi?id=392858
  Every other test passes.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12271

To: sraizada, #ktexteditor
Cc: #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
head7, cullmann, kfunk, sars, dhaumann

D12271: Don't remove trailing whitespace from cursor line

[Subramaniyam Raizada]

sraizada added a comment.


  Would it be appropriate to add this as a config option in the settings? 
Because to me, keeping the sort of whitespace you describe seems like the 
better behavior:
  doSomething(a, b, |
  ^ is correct. But obviously there are differing opinions on this, and a 
setting would resolve that.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12271

To: sraizada, #ktexteditor
Cc: anthonyfieroni, #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham, 
bruns, demsking, head7, cullmann, kfunk, sars, dhaumann

D12295: Supporting nested brackets for Kate autobrackets

[Subramaniyam Raizada]

sraizada created this revision.
sraizada added a reviewer: KTextEditor.
sraizada added a project: KTextEditor.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.
sraizada requested review of this revision.

REVISION SUMMARY
  With autobrackets enabled, Kate adds a closing bracket when opening a 
bracket. E.g. typing "if(" will result in "if(|)" appearing on screen, where | 
is the cursor position. Then, if the closing ) is manually typed in, it gets 
'eaten' by the autogenerated one. So starting from "if(|)", typing in ")" will 
give "if()".
  
  However, this fails when any types of brackets are nested. Typing in "if({})" 
results in the innermost "}" getting eaten, but the closing ")" is not eaten, 
so the resulting text is "if({}))". This is for any combination of 
same/different brackets or quotes, as long as they are nested.
  
  The patch uses a QStack (instead of just a QChar) to keep track of 
this and correctly eat closing brackets. That behaviour does work correctly 
(see attached video - only ({["' and their closing pairs are typed, the cursor 
is not manually moved). However, the stack should be reset when the cursor is 
moved away from the area where brackets are being inserted. I have not been 
able to figure out how to get that to work properly.
  
  On line 1357, upon entering an open bracket/quote, m_currentAutobraceRange 
seems to get set to (cursorposition-1, insertedTextPosition). Or at least 
that's what I think, my C++ knowledge doesn't really go that far, and I'm not 
at all familiar with the codebase or program structure. This needs to take into 
account nesting, instead of just setting the range to cursor position +- 1 unit 
for the bracket.
  
  Sven posted on the kwrite-devel mailing list 
https://mail.kde.org/pipermail/kwrite-devel/2018-April/000346.html that there 
are methods called to move the cursor left or right, as well as a 
cursorPositionChanged event. Hooking into this would allow for a basic 
solution, where moving the cursor at all would clear the autobracket history 
and result in future autobrackets not getting eaten. However, as long as 
Backspace and Enter events don't cause the stack to get cleared, that should 
work for cases when one is just typing a statement that contains multiple 
levels of parentheses in one shot without any errors. I'm a bit busy right now, 
but will try to see if I can implement this or a more comprehensive solution 
within the next few days. I would appreciate any tips on where to look in the 
code, again this is my first experience with C++ in a long, long time and I'm 
just putting code in random places until it works :)
  
  Note: in order to get nested autobraces to get eaten with this patch, line 
3095 has to be commented out, otherwise the stack is cleared upon entering any 
type of closing bracket. Making m_currentAutobracketRange fit the actual range 
instead of one bracket will fix that.
  
  F5810863: recording.mp4 

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12295

AFFECTED FILES
  src/document/katedocument.cpp
  src/document/katedocument.h

To: sraizada, #ktexteditor
Cc: #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
head7, cullmann, kfunk, sars, dhaumann

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

[Subramaniyam Raizada]

sraizada created this revision.
sraizada added a reviewer: KTextEditor.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added a subscriber: Frameworks.
sraizada requested review of this revision.

REVISION SUMMARY
  When at the first autocomplete result, pressing up will wrap around to the 
last result, and vice versa.
  
  This does make test 56 (completion_test) fail with an "Exception: Child 
aborted" - I'm looking into it.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12768

AFFECTED FILES
  src/completion/katecompletiontree.cpp

To: sraizada, #ktexteditor
Cc: #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, cullmann, 
sars, dhaumann

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

[Subramaniyam Raizada]

sraizada added a project: KTextEditor.
sraizada added a subscriber: KTextEditor.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12768

To: sraizada, #ktexteditor
Cc: #ktexteditor, #frameworks, michaelh, kevinapavew, ngraham, bruns, demsking, 
head7, cullmann, kfunk, sars, dhaumann

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

[Subramaniyam Raizada]

sraizada updated this revision to Diff 33884.
sraizada added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Removed unnecessary modifications to code

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12768?vs=33856&id=33884

REVISION DETAIL
  https://phabricator.kde.org/D12768

AFFECTED FILES
  src/completion/katecompletiontree.cpp

To: sraizada, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, brauch, #ktexteditor, michaelh, 
kevinapavew, ngraham, bruns, demsking, head7, cullmann, kfunk, sars, dhaumann, 
#frameworks

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

[Subramaniyam Raizada]

sraizada added a comment.


  I only use Kate/KWrite so didn't test it on the others previously - sorry.
  It actually only works in Kate and KWrite, and has no effect on 
Kile/KDevelop. KDevelop is obviously doing some special stuff, but visually the 
Kile autocompletion seems identical to the one in Kate/KWrite so I'm surprised 
that it doesn't work.
  Any idea what's causing this issue or where I should look in the KDevelop 
code?

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12768

To: sraizada, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, brauch, #ktexteditor, michaelh, 
kevinapavew, ngraham, bruns, demsking, head7, cullmann, kfunk, sars, dhaumann

D12295: Supporting nested brackets for Kate autobrackets

[Subramaniyam Raizada]

sraizada updated this revision to Diff 34333.
sraizada added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Improved patch, closing brackets get eaten properly in all cases I tested.
  
  The only issue I found is that matching bracket highlighting can be off when 
entering 'incorrect' sequences of braces. Such as in this example, where the ) 
in the middle causes the second-to-last parenthesis to be matched with the 
first one.
  F5852484: highlighting.png 
  
  The bracket-eating behaviour with such incorrect sequences is to simple 
ignore the incorrect ) in the middle - all four of the }})) at the end will get 
eaten. Out of the three other text editors I tested (IntelliJ, Sublime Text, 
and 'micro'), Sublime and micro also behave this way.

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12295?vs=32417&id=34333

REVISION DETAIL
  https://phabricator.kde.org/D12295

AFFECTED FILES
  src/document/katedocument.cpp
  src/document/katedocument.h

To: sraizada, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, michaelh, kevinapavew, 
ngraham, bruns, demsking, head7, cullmann, kfunk, sars, dhaumann, #frameworks

D12295: Supporting nested brackets for Kate autobrackets

[Subramaniyam Raizada]

sraizada added a comment.


  The issue with matching unbalanced braces (picture in previous comment) also 
happens in unmodified KWrite, so it doesn't seem to be an issue with this 
revision.
  This fails test 49 - kateview_test; but checking out ktexteditor from git and 
then running make/make install/make test also results in that test failing.
  Thus, I believe this revision is ready for review.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D12295

To: sraizada, #ktexteditor, #kate, cullmann, dhaumann
Cc: ngraham, kwrite-devel, kde-frameworks-devel, #ktexteditor, michaelh, 
kevinapavew, bruns, demsking, head7, cullmann, kfunk, sars, dhaumann