D19785: DocumentPrivate: Review del/backspace

[Christoph Cullmann]

cullmann added a comment.


  Thanks!

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, cullmann
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[loh tar]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:7e739920fd23: Don't delete entire previous line by 
backspace in pos 0 (authored by loh.tar).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D19785?vs=58682&id=58796#toc

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19785?vs=58682&id=58796

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

AFFECTED FILES
  src/document/katedocument.cpp

To: loh.tar, #ktexteditor, cullmann
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[Christoph Cullmann]

cullmann added a comment.


  I ATM have no access to my compile env. here, please commit the fix.
  It would be best to have some test for this, too, for future reference.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, cullmann
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[loh tar]

loh.tar added a comment.


  That may fix it.
  
diff --git a/src/document/katedocument.cpp b/src/document/katedocument.cpp
index e365c380..7c24f67f 100644
--- a/src/document/katedocument.cpp
+++ b/src/document/katedocument.cpp
@@ -3350,7 +3350,9 @@ void 
KTextEditor::DocumentPrivate::backspace(KTextEditor::ViewPrivate *view, con
 
 } else {
 // col == 0: wrap to previous line
-if (line > 0) {
+const Kate::TextLine textLine = m_buffer->plainLine(line - 1);
+
+if (line > 0 && textLine) {
 if (config()->wordWrap() && textLine->endsWith(QLatin1String(" 
"))) {
 // gg: in hard wordwrap mode, backspace must also eat the 
trailing space
 removeText(KTextEditor::Range(line - 1, textLine->length() 
- 1, line, 0));

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, cullmann
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[Christoph Cullmann]

cullmann reopened this revision.
cullmann added a comment.
This revision is now accepted and ready to land.


  See https://bugs.kde.org/show_bug.cgi?id=408016
  
  Guess this needs a revert, + some tests for that cases + a fix, thanks ;=)

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, cullmann
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[loh tar]

This revision was automatically updated to reflect the committed changes.
Closed by commit R39:bcba89914136: Review del/backspace (authored by loh.tar).

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D19785?vs=53955&id=58682

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

AFFECTED FILES
  src/document/katedocument.cpp

To: loh.tar, #ktexteditor, cullmann
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[Christoph Cullmann]

cullmann accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, cullmann
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[loh tar]

loh.tar added a comment.


  I had recently submit a patch which was also affected by RTL text D19621 
. Should this patch here be accepted I can 
upload my current state of the mentioned playing, which works similar as that 
D19621 . It's for me very frustrating to 
test because I never know what is to be expect or what currently happens. Your 
Qt report make it even worse.
  
  Probably can we not much do for a perfect solution as long Qt has some 
serious problem as your Qt bug report says.
  
  BTW I noticed that typing a bracket in RTL text these is shown mirrored, very 
confusing too.
  
  > I'm afraid that the problem isn't with backspace/del, but with the cursor 
positioning
  
  hm, currently is by backspace something to the right removed and by del to 
the left. So I think that's wrong and my patch fix that at least for 
backspace...in most cases IIRC. Then I stopped my playing.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[Safa Alfulaij]

safaalfulaij added a comment.


  Wow! Great :)
  Thanks for looking into this, but I'm afraid that the problem isn't with 
backspace/del, but with the cursor positioning, at least for RTL and mixed text.
  I actually found this to be a problem in Qt after trying to solve it myself 
in KTextEditor: https://bugreports.qt.io/browse/QTBUG-65508
  Not sure what's the root of the problem :(

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: safaalfulaij, kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, 
domson, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[loh tar]

loh.tar added a comment.


  This patch is the byproduct of some playing with RTL text, Bug 385694. 
  But a sane solution may need some more effort, so I tend to not offer my 
current state of that playing.

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D19785: DocumentPrivate: Review del/backspace

[loh tar]

loh.tar created this revision.
loh.tar added a reviewer: KTextEditor.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
loh.tar requested review of this revision.

REVISION SUMMARY
  - Avoid bad selection in some case of undo in block mode
  - Don't try to expand selection in block mode when cursor is in col 0 on 
backspace
  - Improve/fix documentation
  - Code cosmetic

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/document/katedocument.cpp

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, gennad, domson, michaelh, 
ngraham, bruns, demsking, cullmann, sars, dhaumann