--===============1027368456780219592== Content-Type: multipart/alternative; boundary="===============7764367593206367571==" --===============7764367593206367571== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117613/ ----------------------------------------------------------- Review request for Kate, Michal Humpula and Simon St James. Repository: ktexteditor Description ------- I started my "let's split all the tests into tinier pieces" adventure by writing more tests for the replace mode. The final result is this patch, that contains: - More tests. - Fixed a crash: when you typed Ctrl-E (or Ctrl-Y) in an empty document, it crashed. This is because in the first if statement of the commandInsertFromLine function, it should be >= instead of > (such a classic fix :P). - Fixed a crash: when you typed Ctrl-E (or Ctrl-Y) and the cursor was at the end of the line, it crashed. This was fixed by calling doc()->insertText() for this case. Moreover, I took the chance to clean up the commandInsertFromLine function a bit. Most notably, instead of calling the cumbersome getCharAtVirtualColumn function now it will call the doc()->characterAt() function. - I took the chance to clean up the whole KateViReplaceMode class (probably it should've been in another patch, sorry about that :P). Diffs ----- autotests/src/vimode/modes.h 1380b47 autotests/src/vimode/modes.cpp ed7d45b src/vimode/katevireplacemode.h cf67ebb src/vimode/katevireplacemode.cpp 25ac320 Diff: https://git.reviewboard.kde.org/r/117613/diff/ Testing ------- I started by adding more tests to the ReplaceModeTest but I finally decided to split this function into three different functions: - ReplaceBasicTests: for basic replace actions, moving the cursor with the Ctrl key, etc. - ReplaceUndoTests: for actions that want to undo a replacement (e.g. the backspace key). Notice the TODO's here: I'll (hopefully :P) provide another patch in the future to support the Ctrl-W and the Ctrl-U actions. Right now I'm marking this as a TODO. - ReplaceInsertFromLineTests: for the Ctrl-Y and the Ctrl-E actions. All the tests are passing. Thanks, Miquel Sabaté Solà --===============7764367593206367571== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117613/

Review request for Kate, Michal Humpula and Simon St James.
By Miquel Sabaté Solà.
Repository: ktexteditor

Description

I started my "let's split all the tests into tinier pieces" adventure by writing more tests for the replace mode. The final result is this patch, that contains:

- More tests.
- Fixed a crash: when you typed Ctrl-E (or Ctrl-Y) in an empty document, it crashed. This is because in the first if statement of the commandInsertFromLine function, it should be >= instead of > (such a classic fix :P).
- Fixed a crash: when you typed Ctrl-E (or Ctrl-Y) and the cursor was at the end of the line, it crashed. This was fixed by calling doc()->insertText() for this case. Moreover, I took the chance to clean up the commandInsertFromLine function a bit. Most notably, instead of calling the cumbersome getCharAtVirtualColumn function now it will call the doc()->characterAt() function.
- I took the chance to clean up the whole KateViReplaceMode class (probably it should've been in another patch, sorry about that :P).

Testing

I started by adding more tests to the ReplaceModeTest but I finally decided to split this function into three different functions:

- ReplaceBasicTests: for basic replace actions, moving the cursor with the Ctrl key, etc.
- ReplaceUndoTests: for actions that want to undo a replacement (e.g. the backspace key). Notice the TODO's here: I'll (hopefully :P) provide another patch in the future to support the Ctrl-W and the Ctrl-U actions. Right now I'm marking this as a TODO.
- ReplaceInsertFromLineTests: for the Ctrl-Y and the Ctrl-E actions.

All the tests are passing.

Diffs

  • autotests/src/vimode/modes.h (1380b47)
  • autotests/src/vimode/modes.cpp (ed7d45b)
  • src/vimode/katevireplacemode.h (cf67ebb)
  • src/vimode/katevireplacemode.cpp (25ac320)

View Diff

--===============7764367593206367571==-- --===============1027368456780219592== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ KWrite-Devel mailing list KWrite-Devel@kde.org https://mail.kde.org/mailman/listinfo/kwrite-devel --===============1027368456780219592==--