From kwrite-devel Fri Apr 25 21:36:19 2014 From: "Commit Hook" Date: Fri, 25 Apr 2014 21:36:19 +0000 To: kwrite-devel Subject: Re: Review Request 117774: vimode: fixed crash when mapping the u and the U commands. Message-Id: <20140425213619.18037.60291 () probe ! kde ! org> X-MARC-Message: https://marc.info/?l=kwrite-devel&m=139846180110649 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============8730797187195369979==" --===============8730797187195369979== Content-Type: multipart/alternative; boundary="===============1409757147985625317==" --===============1409757147985625317== 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/117774/#review56574 ----------------------------------------------------------- This review has been submitted with commit a2eee0f17e3587a8eefcdb99a333773669324744 by Miquel Sabaté to branch master. - Commit Hook On April 25, 2014, 8:57 p.m., Miquel Sabaté Solà wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/117774/ > ----------------------------------------------------------- > > (Updated April 25, 2014, 8:57 p.m.) > > > Review request for Kate, Michal Humpula and Simon St James. > > > Bugs: 333784 > http://bugs.kde.org/show_bug.cgi?id=333784 > > > Repository: kate > > > Description > ------- > > Right now if you map any key to u or U (or ) and then you execute it, it will crash. I realized about this crash because of the bug report #333784. In this bug report it's said that it behaves incorrectly. However, in master this same case just crashes. I'm not quite sure about this fix, since I don't know much about the undo action in Kate. My reasoning is that when we execute a mapping, we do that by calling editBegin() previously. After the mapping has been executed, we call editEnd(). This is ok except by commands that have to deal with undo/redo actions. So, what I do in this patch is to call editEnd before executing the undo/redo operation so we avoid problems with the previous editBegin(), and we restore to the previous state after executing the undo/redo command by calling editBegin() again. It's a bit hackish, but I think is the best solution here. Note also that this cannot be done in the KateViKeyMapper::executeMapping function since the mapping may contain many other commands that might need the be merged into a single undo-able edit as always. > > > Diffs > ----- > > part/vimode/katevinormalmode.cpp 0be6dda > tests/vimode_test.cpp 171088d > > Diff: https://git.reviewboard.kde.org/r/117774/diff/ > > > Testing > ------- > > All tests are passing. I've also added a couple of tests. > > > Thanks, > > Miquel Sabaté Solà > > --===============1409757147985625317== 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/117774/ 

This review has been submitted with commit a2eee0f17e3587a8eefcdb99a333773669324744 by Miquel Sabaté to branch master.

- Commit Hook


On April 25th, 2014, 8:57 p.m. UTC, Miquel Sabaté Solà wrote:

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

Updated April 25, 2014, 8:57 p.m.

Bugs: 333784
Repository: kate

Description 

Right now if you map any key to u or U (or <c-r>) and then you execute it, it will crash. I realized about this crash because of the bug report #333784. In this bug report it's said that it behaves incorrectly. However, in master this same case just crashes. I'm not quite sure about this fix, since I don't know much about the undo action in Kate. My reasoning is that when we execute a mapping, we do that by calling editBegin() previously. After the mapping has been executed, we call editEnd(). This is ok except by commands that have to deal with undo/redo actions. So, what I do in this patch is to call editEnd before executing the undo/redo operation so we avoid problems with the previous editBegin(), and we restore to the previous state after executing the undo/redo command by calling editBegin() 
 again. It's a bit hackish, but I think is the best solution here. Note also that this cannot be done in the KateViKeyMapper::executeMapping function since the mapping may contain many other commands that might need the be merged into a single undo-able edit as always.

Testing 

All tests are passing. I've also added a couple of tests.

Diffs

  • part/vimode/katevinormalmode.cpp (0be6dda)
  • tests/vimode_test.cpp (171088d)

View Diff
--===============1409757147985625317==-- --===============8730797187195369979== 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 --===============8730797187195369979==--