Merge lp:~kartiksinghal/polly/patch-for-undo-redo-for-compose-box into lp:polly

[Kartik Singhal]

Undo and redo functionality for the Compose box using tiax's python port of gtksourceview's undo mechanism (http://bitbucket.org/tiax/gtk-textbuffer-with-undo/).

[Conscious User (conscioususer)]

Hi Kartik, thanks for the patch.

Those are more "guideline fixes" than actual fixes, so it should be easy:

1) Please move the undobuffer module to the "external" folder and import it with "from polly.external import undobuffer". I do that for all modules copied from other sources.

2) It seems that you are associating undo with z and redo with y. Shouldn't it be ctrl+z and ctrl+y.

3) The GNOME HIG guidelines, which I tend to follow, actually associate redo with shift+ctrl+z.

It is also needed to update the copyright file to reflect the new module, but you can leave that to me.

review: Needs Fixing

[Kartik Singhal (kartiksinghal)]

On Sat, Jun 9, 2012 at 9:59 PM, Conscious User <email address hidden>wrote:

> Review: Needs Fixing
>
> Hi Kartik, thanks for the patch.
>

Hi, thanks for the review. :)

> Those are more "guideline fixes" than actual fixes, so it should be easy:
>
> 1) Please move the undobuffer module to the "external" folder and import
> it with "from polly.external import undobuffer". I do that for all modules
> copied from other sources.
>

Okay. Updating this.

> 2) It seems that you are associating undo with z and redo with y.
> Shouldn't it be ctrl+z and ctrl+y.
>

No, they are correctly associated to Ctrl+z and y respectively.
Gtk.keysyms.z maps to Ctrl+z itself.

> 3) The GNOME HIG guidelines, which I tend to follow, actually associate
> redo with shift+ctrl+z.
>

I had this doubt - what shortcut to use for redo while associating Ctrl+y.
I wasn't aware about the GNOME HIG, fixing.

> It is also needed to update the copyright file to reflect the new module,
> but you can leave that to me.
>

Sure.

--
Kartik
http://k4rtik.wordpress.com/

[Conscious User (conscioususer)]

Thanks again. :) Hopefully we are almost there, here's what needs fixing this time:

1) In the external __init__ file, replace "import oauth2, wraplabel" with "import oauth2, wraplabel, undobuffer"

2) For consistency, please add the "from polly.external" line as the second polly import, right above "from polly.shortener"

3) As I warned, the undo is working when you just type "z", without the Ctrl. Use Gdk.CONTROL_MASK to ensure Ctrl is pressed.

4) Also use Gdk.SHIFT_MASK to ensure shift is pressed. It's more robust than using capital "Z".

Oh, and do not make new commits just for fixing. Revert the previous commits and push the branch with --overwrite.

review: Needs Fixing

[Conscious User (conscioususer)]

Sorry it took this long. Only very small things now:

1) I don't think considering both z and Z is necessary.

2) Please use multiple "=="s instead of using "in". Yes, I'm aware this is less pythonic, but it's still better than inconsistency in a single point.

3) There is an extra linebreak at the end, after the redo method.

I promise the next version will not take more than one day!

review: Needs Fixing

[Conscious User (conscioususer)]

By the way, branch the latest version, as there was a small change since your last proposal.

Unmerged revisions

402. By Kartik Singhal on 2012-08-03

added undo/redo feature in compose box

=== modified file 'src/polly/external/__init__.py'
--- src/polly/external/__init__.py	2011-08-24 12:29:48 +0000
+++ src/polly/external/__init__.py	2012-08-03 05:52:20 +0000
@@ -19,4 +19,4 @@
 
 
 
-import oauth2, wraplabel
+import oauth2, wraplabel, undobuffer

=== added file 'src/polly/external/undobuffer.py'
--- src/polly/external/undobuffer.py	1970-01-01 00:00:00 +0000
+++ src/polly/external/undobuffer.py	2012-08-03 05:52:20 +0000
@@ -0,0 +1,234 @@
+#!/usr/bin/env python
+# -*- coding:utf-8 -*-
+
+""" gtk textbuffer with undo functionality """
+
+#Copyright (C) 2009 Florian Heinle
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+
+import gtk
+
+class UndoableInsert(object):
+    """something that has been inserted into our textbuffer"""
+    def __init__(self, text_iter, text, length):
+        self.offset = text_iter.get_offset()
+        self.text = text
+        self.length = length
+        if self.length > 1 or self.text in ("\r", "\n", " "):
+            self.mergeable = False
+        else:
+            self.mergeable = True
+
+class UndoableDelete(object):
+    """something that has ben deleted from our textbuffer"""
+    def __init__(self, text_buffer, start_iter, end_iter):
+        self.text = text_buffer.get_text(start_iter, end_iter)
+        self.start = start_iter.get_offset()
+        self.end = end_iter.get_offset()
+        # need to find out if backspace or delete key has been used
+        # so we don't mess up during redo
+        insert_iter = text_buffer.get_iter_at_mark(text_buffer.get_insert())
+        if insert_iter.get_offset() <= self.start:
+            self.delete_key_used = True
+        else:
+            self.delete_key_used = False
+        if self.end - self.start > 1 or self.text in ("\r", "\n", " "):
+            self.mergeable = False
+        else:
+            self.mergeable = True
+
+class UndoableBuffer(gtk.TextBuffer):
+    """text buffer with added undo capabilities
+
+    designed as a drop-in replacement for gtksourceview,
+    at least as far as undo is concerned"""
+    
+    def __init__(self):
+        """
+        we'll need empty stacks for undo/redo and some state keeping
+        """
+        gtk.TextBuffer.__init__(self)
+        self.undo_stack = []
+        self.redo_stack = []
+        self.not_undoable_action = False
+        self.undo_in_progress = False
+        self.connect('insert-text', self.on_insert_text)
+        self.connect('delete-range', self.on_delete_range)
+
+    @property
+    def can_undo(self):
+        return bool(self.undo_stack)
+
+    @property
+    def can_redo(self):
+        return bool(self.redo_stack)
+
+    def on_insert_text(self, textbuffer, text_iter, text, length):
+        def can_be_merged(prev, cur):
+            """see if we can merge multiple inserts here
+
+            will try to merge words or whitespace
+            can't merge if prev and cur are not mergeable in the first place
+            can't merge when user set the input bar somewhere else
+            can't merge across word boundaries"""
+            WHITESPACE = (' ', '\t')
+            if not cur.mergeable or not prev.mergeable:
+                return False
+            elif cur.offset != (prev.offset + prev.length):
+                return False
+            elif cur.text in WHITESPACE and not prev.text in WHITESPACE:
+                return False
+            elif prev.text in WHITESPACE and not cur.text in WHITESPACE:
+                return False
+            return True
+
+        if not self.undo_in_progress:
+            self.redo_stack = []
+        if self.not_undoable_action:
+            return
+        undo_action = UndoableInsert(text_iter, text, length)
+        try:
+            prev_insert = self.undo_stack.pop()
+        except IndexError:
+            self.undo_stack.append(undo_action)
+            return
+        if not isinstance(prev_insert, UndoableInsert):
+            self.undo_stack.append(prev_insert)
+            self.undo_stack.append(undo_action)
+            return
+        if can_be_merged(prev_insert, undo_action):
+            prev_insert.length += undo_action.length
+            prev_insert.text += undo_action.text
+            self.undo_stack.append(prev_insert)
+        else:
+            self.undo_stack.append(prev_insert)
+            self.undo_stack.append(undo_action)
+        
+    def on_delete_range(self, text_buffer, start_iter, end_iter):
+        def can_be_merged(prev, cur):
+            """see if we can merge multiple deletions here
+
+            will try to merge words or whitespace
+            can't merge if prev and cur are not mergeable in the first place
+            can't merge if delete and backspace key were both used
+            can't merge across word boundaries"""
+
+            WHITESPACE = (' ', '\t')
+            if not cur.mergeable or not prev.mergeable:
+                return False
+            elif prev.delete_key_used != cur.delete_key_used:
+                return False
+            elif prev.start != cur.start and prev.start != cur.end:
+                return False
+            elif cur.text not in WHITESPACE and \
+               prev.text in WHITESPACE:
+                return False
+            elif cur.text in WHITESPACE and \
+               prev.text not in WHITESPACE:
+                return False
+            return True
+
+        if not self.undo_in_progress:
+            self.redo_stack = []
+        if self.not_undoable_action:
+            return
+        undo_action = UndoableDelete(text_buffer, start_iter, end_iter)
+        try:
+            prev_delete = self.undo_stack.pop()
+        except IndexError:
+            self.undo_stack.append(undo_action)
+            return
+        if not isinstance(prev_delete, UndoableDelete):
+            self.undo_stack.append(prev_delete)
+            self.undo_stack.append(undo_action)
+            return
+        if can_be_merged(prev_delete, undo_action):
+            if prev_delete.start == undo_action.start: # delete key used
+                prev_delete.text += undo_action.text
+                prev_delete.end += (undo_action.end - undo_action.start)
+            else: # Backspace used
+                prev_delete.text = "%s%s" % (undo_action.text,
+                                                     prev_delete.text)
+                prev_delete.start = undo_action.start
+            self.undo_stack.append(prev_delete)
+        else:
+            self.undo_stack.append(prev_delete)
+            self.undo_stack.append(undo_action)
+
+    def begin_not_undoable_action(self):
+        """don't record the next actions
+        
+        toggles self.not_undoable_action"""
+        self.not_undoable_action = True        
+
+    def end_not_undoable_action(self):
+        """record next actions
+        
+        toggles self.not_undoable_action"""
+        self.not_undoable_action = False
+    
+    def undo(self):
+        """undo inserts or deletions
+
+        undone actions are being moved to redo stack"""
+        if not self.undo_stack:
+            return
+        self.begin_not_undoable_action()
+        self.undo_in_progress = True
+        undo_action = self.undo_stack.pop()
+        self.redo_stack.append(undo_action)
+        if isinstance(undo_action, UndoableInsert):
+            start = self.get_iter_at_offset(undo_action.offset)
+            stop = self.get_iter_at_offset(
+                undo_action.offset + undo_action.length
+            )
+            self.delete(start, stop)
+            self.place_cursor(start)
+        else:
+            start = self.get_iter_at_offset(undo_action.start)
+            self.insert(start, undo_action.text)
+            stop = self.get_iter_at_offset(undo_action.end)
+            if undo_action.delete_key_used:
+                self.place_cursor(start)
+            else:
+                self.place_cursor(stop)
+        self.end_not_undoable_action()
+        self.undo_in_progress = False
+
+    def redo(self):
+        """redo inserts or deletions
+
+        redone actions are moved to undo stack"""
+        if not self.redo_stack:
+            return
+        self.begin_not_undoable_action()
+        self.undo_in_progress = True
+        redo_action = self.redo_stack.pop()
+        self.undo_stack.append(redo_action)
+        if isinstance(redo_action, UndoableInsert):
+            start = self.get_iter_at_offset(redo_action.offset)
+            self.insert(start, redo_action.text)
+            new_cursor_pos = self.get_iter_at_offset(
+                redo_action.offset + redo_action.length
+            )
+            self.place_cursor(new_cursor_pos)
+        else:
+            start = self.get_iter_at_offset(redo_action.start)
+            stop = self.get_iter_at_offset(redo_action.end)
+            self.delete(start, stop)
+            self.place_cursor(start)
+        self.end_not_undoable_action()
+        self.undo_in_progress = False

=== modified file 'src/polly/gui/header/entry.py'
--- src/polly/gui/header/entry.py	2011-10-30 01:15:58 +0000
+++ src/polly/gui/header/entry.py	2012-08-03 05:52:20 +0000
@@ -39,6 +39,8 @@
 
 from polly import ENCODING, dispatch
 
+from polly.external import undobuffer
+
 from polly.shortener import Exit, Error
 
 from polly.gui import NICK_DIMENSION, run_generic_error_dialog
@@ -461,6 +463,14 @@
                 self.completer.end()
 
                 return True
+        elif event.keyval in (Gtk.keysyms.z, Gtk.keysyms.Z) and \
+                (event.state & Gtk.gdk.CONTROL_MASK) != 0 and \
+                (event.state & Gtk.gdk.SHIFT_MASK) == 0:
+            self.undo()
+        elif event.keyval in (Gtk.keysyms.z, Gtk.keysyms.Z) and \
+                (event.state & Gtk.gdk.CONTROL_MASK) != 0 and \
+                (event.state & Gtk.gdk.SHIFT_MASK) != 0:
+            self.redo()
         elif (event.keyval == Gtk.keysyms.KP_Left or event.keyval == Gtk.keysyms.Left or
               event.keyval == Gtk.keysyms.KP_Right or event.keyval == Gtk.keysyms.Right or
               event.keyval == Gtk.keysyms.Escape):
@@ -728,7 +738,7 @@
         self.texttag = Gtk.TextTag(None)
         self.texttag.set_property(u'underline', Pango.UNDERLINE_SINGLE)
 
-        self.textview = Gtk.TextView(None)
+        self.textview = Gtk.TextView(undobuffer.UndoableBuffer())   # now we use Undo-able buffer
         self.textview.set_wrap_mode(Gtk.WRAP_WORD_CHAR)
         self.textview.connect(u'button-press-event', self._on_textview_button_press_event)
         self.textview.connect(u'focus-in-event', self._on_textview_focus_in_event)
@@ -919,3 +929,14 @@
     def move_completer(self):
         if self.nicks:
             self._move_completer()
+
+
+    def undo(self):
+        if self.textbuffer.can_undo:
+            self.textbuffer.undo()
+
+
+    def redo(self):
+        if self.textbuffer.can_redo:
+            self.textbuffer.redo()
+