Merge lp:~sinzui/bzr-gtk/gpush into lp:bzr-gtk

[Curtis Hovey]

Integrate the pogress bar and message into the push dialog.

    Launchpad bug: https://bugs.launchpad.net/bugs/113660
    Pre-implementation: jelmer

This branch replaces the progress dialog with a progress panel and
the info dialog with a message label. When the user choosed to push,
the progress panel is shown, and the message is shown when completed.

--------------------------------------------------------------------

RULES

    * Add a progress panel and a label to the push dialog that are not
      visible until the push starts.
    * Update HBox to Box.
    * Remove the unused attrs and variables from the code.
      * I decided not to remove two unused attributes because it breaks
        anything that calls PushDialog. Do we want to change the API?
    * Fix the spacing and padding to conform to the GNOME Human
      Interface guidelines.
    * ADDENDUM: I made a minor change to do_push to return the message
      instead of the count. PushResult.__int__ is deprecated. I decided
      I want to fix do_push() in my next branch without changing
      _on_push_clicked().

QA

    http://people.canonical.com/~curtis/push-progress.png
    http://people.canonical.com/~curtis/push-complete.png

    Run this command from a branch with theses changes.
    BZR_PLUGINS_AT=gtk@./ bzr gpush

[lp:~sinzui/bzr-gtk/gpush]

793. By Curtis Hovey on 2012-03-04

Merged trunk.

794. By Curtis Hovey on 2012-03-04

Fixed spelling.

[Jelmer Vernooij (jelmer)]

Thanks, very nice.

review: Approve

=== modified file 'push.py'
--- push.py	2012-02-27 18:47:21 +0000
+++ push.py	2012-03-04 02:24:17 +0000
@@ -1,4 +1,5 @@
-# Copyright (C) 2006 by Szilveszter Farkas (Phanatic) <[e-mail]>
+# Copyright (C) 2006 by Szilveszter Farkas (Phanatic)
+#                       <[e-mail]>
 # Copyright (C) 2007 by Jelmer Vernooij <[e-mail]>
 #
 # This program is free software; you can redistribute it and/or modify
@@ -15,20 +16,23 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-from gi.repository import GObject
 from gi.repository import Gtk
 
 from errors import show_bzr_error
 
-import bzrlib.errors as errors
+from bzrlib import (
+    errors,
+    ui,
+    )
+from bzrlib.bzrdir import BzrDir
+from bzrlib.transport import get_transport
 
 from bzrlib.plugins.gtk.dialog import (
-    info_dialog,
     question_dialog,
     )
-
 from bzrlib.plugins.gtk.history import UrlHistory
 from bzrlib.plugins.gtk.i18n import _i18n
+from bzrlib.plugins.gtk.ui import ProgressPanel
 
 
 class PushDialog(Gtk.Dialog):
@@ -37,39 +41,43 @@
     def __init__(self, repository, revid, branch=None, parent=None):
         """Initialize the Push dialog. """
         super(PushDialog, self).__init__(
-            title="Push", parent=parent, flags=0,
-            buttons=(Gtk.STOCK_CANCEL, Gtk.ResponseType.CANCEL))
-
-
-        # Get arguments
+            title="Push", parent=parent, flags=0, border_width=6,
+            buttons=(Gtk.STOCK_CLOSE, Gtk.ResponseType.CLOSE))
+        self.branch = branch
+
+        # Unused arguments
         self.repository = repository
         self.revid = revid
-        self.branch = branch
 
         # Create the widgets
         self._label_location = Gtk.Label(label=_i18n("Location:"))
         self._combo = Gtk.ComboBox.new_with_entry()
         self._button_push = Gtk.Button(_i18n("_Push"), use_underline=True)
-        self._hbox_location = Gtk.HBox()
+        self._hbox_location = Gtk.Box(Gtk.Orientation.HORIZONTAL, 6)
+        self._push_message = Gtk.Label(xalign=0)
+        self._progress_widget = ProgressPanel()
 
         # Set callbacks
+        ui.ui_factory.set_progress_bar_widget(self._progress_widget)
+        self.connect('close', self._on_close_clicked)
         self._button_push.connect('clicked', self._on_push_clicked)
 
         # Set properties
-        self._label_location.set_alignment(0, 0.5)
-        self._hbox_location.set_spacing(3)
-        self.get_content_area().set_spacing(3)
+        content_area = self.get_content_area()
+        content_area.set_spacing(6)
 
         # Pack widgets
-        self._hbox_location.pack_start(
-            self._label_location, False, False, 0)
-        self._hbox_location.pack_start(self._combo, True, True, 0)
-        self.get_content_area().pack_start(self._hbox_location, True, True, 0)
-        # XXX sinzui 2011-08-12: maybe False, False, 0
+        self._hbox_location.pack_start(self._label_location, False, False, 0)
+        self._hbox_location.pack_start(self._combo, False, False, 0)
+        content_area.pack_start(self._hbox_location, True, True, 0)
+        content_area.pack_start(self._progress_widget, True, True, 0)
+        content_area.pack_start(self._push_message, True, True, 0)
         self.get_action_area().pack_end(self._button_push, True, True, 0)
 
         # Show the dialog
-        self.get_content_area().show_all()
+        content_area.show_all()
+        self._progress_widget.hide()
+        self._push_message.hide()
 
         # Build location history
         self._history = UrlHistory(self.branch.get_config(), 'push_history')
@@ -79,7 +87,7 @@
         """Build up the location history. """
         self._combo_model = Gtk.ListStore(str)
         for item in self._history.get_entries():
-            self._combo_model.append([ item ])
+            self._combo_model.append([item])
         self._combo.set_model(self._combo_model)
         self._combo.set_entry_text_column(0)
 
@@ -88,28 +96,36 @@
             if location is not None:
                 self._combo.get_child().set_text(location)
 
+    def _on_close_clicked(self, widget):
+        """Close dialog handler."""
+        ui.ui_factory.set_progress_bar_widget(None)
+
     @show_bzr_error
     def _on_push_clicked(self, widget):
         """Push button clicked handler. """
+        self._push_message.hide()
+        self._progress_widget.tick()
         location = self._combo.get_child().get_text()
-        revs = 0
 
         try:
-            revs = do_push(self.branch, location=location, overwrite=False)
+            message = do_push(self.branch, location, overwrite=False)
         except errors.DivergedBranches:
-            response = question_dialog(_i18n('Branches have been diverged'),
-                                       _i18n('You cannot push if branches have diverged.\nOverwrite?'))
+            response = question_dialog(
+                _i18n('Branches have been diverged'),
+                _i18n('You cannot push if branches have diverged.\n'
+                      'Overwrite?'))
             if response == Gtk.ResponseType.YES:
-                revs = do_push(self.branch, location=location, overwrite=True)
-
-        if self.branch is not None and self.branch.get_push_location() is None:
+                message = do_push(self.branch, location, overwrite=True)
+            else:
+                return
+        self._history.add_entry(location)
+        if (self.branch is not None
+            and self.branch.get_push_location() is None):
             self.branch.set_push_location(location)
-
-        self._history.add_entry(location)
-        info_dialog(_i18n('Push successful'),
-                    _i18n("%d revision(s) pushed.") % revs)
-
-        self.response(Gtk.ResponseType.OK)
+        if message:
+            self._progress_widget.finished()
+            self._push_message.props.label = message
+            self._push_message.show()
 
 
 def do_push(br_from, location, overwrite):
@@ -120,14 +136,9 @@
     :param overwrite: overwrite target location if it diverged
     :return: number of revisions pushed
     """
-    from bzrlib.bzrdir import BzrDir
-    from bzrlib.transport import get_transport
-
     transport = get_transport(location)
     location_url = transport.base
 
-    old_rh = []
-
     try:
         dir_to = BzrDir.open(location_url)
         br_to = dir_to.open_branch()
@@ -138,8 +149,10 @@
             relurl = transport.relpath(location_url)
             transport.mkdir(relurl)
         except errors.NoSuchFile:
-            response = question_dialog(_i18n('Non existing parent directory'),
-                         _i18n("The parent directory (%s)\ndoesn't exist. Create?") % location)
+            response = question_dialog(
+                _i18n('Non existing parent directory'),
+                _i18n("The parent directory (%s)\ndoesn't exist. Create?") %
+                location)
             if response == Gtk.ResponseType.OK:
                 transport.create_prefix()
             else:
@@ -149,7 +162,7 @@
         br_to = dir_to.open_branch()
         count = len(br_to.revision_history())
     else:
-        old_rh = br_to.revision_history()
+        br_to.revision_history()
         try:
             tree_to = dir_to.open_workingtree()
         except errors.NotLocalUrl:
@@ -160,4 +173,6 @@
         else:
             count = tree_to.pull(br_from, overwrite)
 
-    return count
+    # The count var is either an int or a PushResult. PushResult is being
+    # coerced into an int, but the method is deprecated.
+    return _i18n("%d revision(s) pushed.") % int(count)

=== modified file 'tests/__init__.py'
--- tests/__init__.py	2012-02-28 17:52:54 +0000
+++ tests/__init__.py	2012-03-04 02:24:17 +0000
@@ -46,23 +46,28 @@
 class MockMethod(object):
 
     @classmethod
-    def bind(klass, test_instance, obj, method_name, return_value=None):
+    def bind(klass, test_instance, obj, method_name,
+             return_value=None, raise_error=None, raise_on=1):
         original_method = getattr(obj, method_name)
         test_instance.addCleanup(setattr, obj, method_name, original_method)
-        setattr(obj, method_name, klass(return_value))
+        setattr(obj, method_name, klass(return_value, raise_error, raise_on))
 
-    def __init__(self, return_value=None):
+    def __init__(self, return_value=None, raise_error=None, raise_on=1):
         self.called = False
         self.call_count = 0
         self.args = None
         self.kwargs = None
         self.return_value = return_value
+        self.raise_error = raise_error
+        self.raise_on = raise_on
 
     def __call__(self, *args, **kwargs):
         self.called = True
         self.call_count += 1
         self.args = args
         self.kwargs = kwargs
+        if self.raise_error is not None and self.call_count == self.raise_on:
+            raise self.raise_error
         return self.return_value
 
 

=== added file 'tests/test_push.py'
--- tests/test_push.py	1970-01-01 00:00:00 +0000
+++ tests/test_push.py	2012-03-04 02:24:17 +0000
@@ -0,0 +1,135 @@
+# Copyright (C) 2012 Curtis Hovey <[e-mail]>
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+"""Test the ui functionality."""
+
+from gi.repository import Gtk
+
+from bzrlib import (
+    errors,
+    tests,
+    ui,
+    )
+
+from bzrlib.plugins.gtk import (
+    push,
+    set_ui_factory,
+    )
+from bzrlib.plugins.gtk.tests import MockMethod
+from bzrlib.plugins.gtk.history import UrlHistory
+from bzrlib.plugins.gtk.ui import ProgressPanel
+
+
+class PushTestCase(tests.TestCaseWithMemoryTransport):
+
+    def make_push_branch(self):
+        tree = self.make_branch_and_memory_tree('test')
+        return tree.branch
+
+    def test_init(self):
+        set_ui_factory()
+        branch = self.make_push_branch()
+        dialog = push.PushDialog(
+            repository=None, revid=None, branch=branch, parent=None)
+        self.assertIs(None, dialog.props.parent)
+        self.assertIs(None, dialog.repository)
+        self.assertIs(None, dialog.revid)
+        self.assertIs(branch, dialog.branch)
+        # Layout rules to match HIG.
+        self.assertIsInstance(dialog._label_location, Gtk.Label)
+        self.assertIsInstance(dialog._combo, Gtk.ComboBox)
+        self.assertIsInstance(dialog._button_push, Gtk.Button)
+        self.assertIsInstance(dialog._hbox_location, Gtk.Box)
+        self.assertIsInstance(dialog._progress_widget, ProgressPanel)
+        self.assertIsInstance(dialog._push_message, Gtk.Label)
+        self.assertIsInstance(dialog._history, UrlHistory)
+        self.assertIs(
+            ui.ui_factory._progress_bar_widget, dialog._progress_widget)
+        self.assertEqual(
+            Gtk.Orientation.HORIZONTAL,
+            dialog._hbox_location.props.orientation)
+        self.assertEqual(0.0, dialog._push_message.props.xalign)
+        self.assertEqual(6, dialog.props.border_width)
+        self.assertEqual(6, dialog._hbox_location.props.spacing)
+        self.assertEqual(6, dialog.get_content_area().props.spacing)
+        # Show rules.
+        self.assertIs(True, dialog._combo.props.visible)
+        self.assertIs(False, dialog._progress_widget.props.visible)
+        self.assertIs(False, dialog._push_message.props.visible)
+
+    def test_build_history(self):
+        set_ui_factory()
+        branch = self.make_push_branch()
+        branch.set_push_location('lp:~user/fnord/trunk')
+        dialog = push.PushDialog(None, None, branch)
+        dialog._history.add_entry('lp:~user/fnord/test1')
+        dialog._history.add_entry('lp:~user/fnord/test2')
+        dialog._build_history()
+        self.assertEqual(
+            'lp:~user/fnord/trunk', dialog._combo.get_child().props.text)
+        self.assertIsInstance(dialog._combo_model, Gtk.ListStore)
+        self.assertIs(dialog._combo.get_model(), dialog._combo_model)
+        locations = [row[0] for row in dialog._combo_model]
+        self.assertEqual(
+            ['lp:~user/fnord/test1', 'lp:~user/fnord/test2'], locations)
+
+    def test_on_close_clicked(self):
+        # The ui_factory's progress bar widget is set to None.
+        set_ui_factory()
+        branch = self.make_push_branch()
+        dialog = push.PushDialog(None, None, branch)
+        dialog._on_close_clicked(None)
+        self.assertIs(None, ui.ui_factory._progress_bar_widget)
+
+    def test_on_push_clicked_without_errors(self):
+        # Verify the dialog's and branch's final states after a push.
+        MockMethod.bind(self, push, 'do_push', "test success")
+        set_ui_factory()
+        branch = self.make_push_branch()
+        dialog = push.PushDialog(None, None, branch)
+        MockMethod.bind(self, dialog._progress_widget, 'tick')
+        dialog._combo.get_child().props.text = 'lp:~user/fnord/test'
+        dialog._on_push_clicked(None)
+        self.assertIs(True, dialog._progress_widget.tick.called)
+        self.assertIs(False, dialog._progress_widget.props.visible)
+        self.assertIs(True, push.do_push.called)
+        self.assertEqual(
+            (branch, 'lp:~user/fnord/test'), push.do_push.args)
+        self.assertEqual(
+            {'overwrite': False}, push.do_push.kwargs)
+        self.assertIs(True, dialog._push_message.props.visible)
+        self.assertEqual('test success', dialog._push_message.props.label)
+        self.assertEqual(
+            'lp:~user/fnord/test', dialog._history.get_entries()[-1])
+        self.assertEqual('lp:~user/fnord/test', branch.get_push_location())
+
+    def test_on_push_clicked_with_divered_branches(self):
+        # Verify that when DivergedBranches is raise, the user can choose
+        # to overwrite the branch.
+        error = errors.DivergedBranches(None, None)
+        MockMethod.bind(self, push, 'do_push', raise_error=error)
+        MockMethod.bind(self, push, 'question_dialog', Gtk.ResponseType.YES)
+        set_ui_factory()
+        branch = self.make_push_branch()
+        dialog = push.PushDialog(None, None, branch)
+        dialog._combo.get_child().props.text = 'lp:~user/fnord/test'
+        dialog._on_push_clicked(None)
+        self.assertIs(True, push.do_push.called)
+        self.assertEqual(2, push.do_push.call_count)
+        self.assertEqual(
+            (branch, 'lp:~user/fnord/test'), push.do_push.args)
+        self.assertEqual(
+            {'overwrite': True}, push.do_push.kwargs)