Rev 5426: (mbp) add structured confirmations to the ui_factory (Martin Pool) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Wed Sep 15 13:02:37 BST 2010


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5426 [merge]
revision-id: pqm at pqm.ubuntu.com-20100915120235-gjrl0gmv0vxbu7b0
parent: pqm at pqm.ubuntu.com-20100914163122-geipr89d2myl2299
parent: mbp at sourcefrog.net-20100915104051-pkjmuc2bc27buysp
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2010-09-15 13:02:35 +0100
message:
  (mbp) add structured confirmations to the ui_factory (Martin Pool)
modified:
  bzrlib/builtins.py             builtins.py-20050830033751-fc01482b9ca23183
  bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
  bzrlib/msgeditor.py            msgeditor.py-20050901111708-ef6d8de98f5d8f2f
  bzrlib/tests/blackbox/test_uncommit.py test_uncommit.py-20051027212835-84944b63adae51be
  bzrlib/tests/per_uifactory/__init__.py __init__.py-20090923045301-o12zypjwsidxn2hy-1
  bzrlib/tests/test_ui.py        test_ui.py-20051130162854-458e667a7414af09
  bzrlib/ui/__init__.py          ui.py-20050824083933-8cf663c763ba53a9
  doc/developers/ui.txt          ui.txt-20100903090119-enuzuixmeqq0b2p0-1
=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py	2010-09-14 09:29:57 +0000
+++ b/bzrlib/builtins.py	2010-09-15 10:40:51 +0000
@@ -4801,8 +4801,11 @@
             self.outf.write('The above revision(s) will be removed.\n')
 
         if not force:
-            if not ui.ui_factory.get_boolean('Are you sure'):
-                self.outf.write('Canceled')
+            if not ui.ui_factory.confirm_action(
+                    'Uncommit these revisions',
+                    'bzrlib.builtins.uncommit',
+                    {}):
+                self.outf.write('Canceled\n')
                 return 0
 
         mutter('Uncommitting from {%s} to {%s}',

=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py	2010-09-13 02:32:44 +0000
+++ b/bzrlib/lockdir.py	2010-09-15 10:40:51 +0000
@@ -358,7 +358,9 @@
             return
         if holder_info is not None:
             lock_info = '\n'.join(self._format_lock_info(holder_info))
-            if bzrlib.ui.ui_factory.get_boolean("Break %s" % lock_info):
+            if bzrlib.ui.ui_factory.confirm_action(
+                "Break %(lock_info)s", 'bzrlib.lockdir.break', 
+                dict(lock_info=lock_info)):
                 self.force_break(holder_info)
 
     def force_break(self, dead_holder_info):

=== modified file 'bzrlib/msgeditor.py'
--- a/bzrlib/msgeditor.py	2010-04-30 11:35:43 +0000
+++ b/bzrlib/msgeditor.py	2010-09-13 10:04:19 +0000
@@ -149,8 +149,10 @@
             return None
         edited_content = msg_transport.get_bytes(basename)
         if edited_content == reference_content:
-            if not ui.ui_factory.get_boolean(
-                "Commit message was not edited, use anyway"):
+            if not ui.ui_factory.confirm_action(
+                "Commit message was not edited, use anyway",
+                "bzrlib.msgeditor.unchanged",
+                {}):
                 # Returning "" makes cmd_commit raise 'empty commit message
                 # specified' which is a reasonable error, given the user has
                 # rejected using the unedited template.

=== modified file 'bzrlib/tests/blackbox/test_uncommit.py'
--- a/bzrlib/tests/blackbox/test_uncommit.py	2010-03-01 16:18:43 +0000
+++ b/bzrlib/tests/blackbox/test_uncommit.py	2010-09-15 09:35:42 +0000
@@ -22,7 +22,10 @@
 from bzrlib.bzrdir import BzrDirMetaFormat1
 from bzrlib.errors import BzrError, BoundBranchOutOfDate
 from bzrlib.tests import TestCaseWithTransport
-from bzrlib.tests.script import ScriptRunner
+from bzrlib.tests.script import (
+    run_script,
+    ScriptRunner,
+    )
 
 
 class TestUncommit(TestCaseWithTransport):
@@ -61,6 +64,20 @@
         out, err = self.run_bzr('status')
         self.assertEquals(out, 'modified:\n  a\n')
 
+    def test_uncommit_interactive(self):
+        """Uncommit seeks confirmation, and doesn't proceed without it."""
+        wt = self.create_simple_tree()
+        os.chdir('tree')
+        run_script(self, """    
+        $ bzr uncommit
+        ...
+        The above revision(s) will be removed.
+        2>Uncommit these revisions? [y/n]: 
+        <n
+        Canceled
+        """)
+        self.assertEqual(['a2'], wt.get_parent_ids())
+
     def test_uncommit_no_history(self):
         wt = self.make_branch_and_tree('tree')
         out, err = self.run_bzr('uncommit --force', retcode=1)

=== modified file 'bzrlib/tests/per_uifactory/__init__.py'
--- a/bzrlib/tests/per_uifactory/__init__.py	2010-02-23 07:43:11 +0000
+++ b/bzrlib/tests/per_uifactory/__init__.py	2010-09-09 07:49:00 +0000
@@ -62,6 +62,18 @@
         self.factory.be_quiet(False)
         self.assertEquals(False, self.factory.is_quiet())
 
+    def test_confirm_action(self):
+        # confirm_action should be answered by every ui factory; even
+        # noninteractive ones should have a reasonable default
+        self._load_responses([True])
+        result = self.factory.confirm_action(
+            'Break a lock?',
+            'bzr.lock.break.confirm',
+            {})
+        # will be true either because we read it from the input or because
+        # that's the default
+        self.assertEquals(result, True)
+
     def test_note(self):
         self.factory.note("a note to the user")
         self._check_note("a note to the user")
@@ -150,6 +162,11 @@
         # Without a TTY, we shouldn't display anything
         self.assertEqual('', self.stderr.getvalue())
 
+    def _load_responses(self, responses):
+        self.factory.stdin.seek(0)
+        self.factory.stdin.writelines([(r and "y\n" or "n\n") for r in responses])
+        self.factory.stdin.seek(0)
+
 
 class TestTTYTextUIFactory(TestTextUIFactory):
 
@@ -222,6 +239,9 @@
     def _check_log_transport_activity_display_no_bytes(self):
         pass
 
+    def _load_responses(self, responses):
+        pass
+
 
 class TestCannedInputUIFactory(tests.TestCase, UIFactoryTestMixin):
     # discards output, reads input from variables
@@ -250,3 +270,6 @@
 
     def _check_log_transport_activity_display_no_bytes(self):
         pass
+
+    def _load_responses(self, responses):
+        self.factory.responses.extend(responses)

=== modified file 'bzrlib/tests/test_ui.py'
--- a/bzrlib/tests/test_ui.py	2010-09-14 08:14:22 +0000
+++ b/bzrlib/tests/test_ui.py	2010-09-15 10:40:51 +0000
@@ -18,11 +18,12 @@
 """
 
 import os
-import re
 import time
 
 from StringIO import StringIO
 
+from testtools.matchers import *
+
 from bzrlib import (
     config,
     errors,
@@ -53,16 +54,28 @@
         ui = tests.TestUIFactory(stdin=None,
             stdout=tests.StringIOWrapper(),
             stderr=tests.StringIOWrapper())
-        os = ui.make_output_stream()
-        self.assertEquals(os.encoding, enc)
+        output = ui.make_output_stream()
+        self.assertEquals(output.encoding, enc)
 
 
 class TestTextUIFactory(tests.TestCase):
 
+    def make_test_ui_factory(self, stdin_contents):
+        ui = tests.TestUIFactory(stdin=stdin_contents,
+                                 stdout=tests.StringIOWrapper(),
+                                 stderr=tests.StringIOWrapper())
+        return ui
+
+    def test_text_factory_confirm(self):
+        # turns into reading a regular boolean
+        ui = self.make_test_ui_factory('n\n')
+        self.assertEquals(ui.confirm_action('Should %(thing)s pass?',
+            'bzrlib.tests.test_ui.confirmation',
+            {'thing': 'this'},),
+            False)
+
     def test_text_factory_ascii_password(self):
-        ui = tests.TestUIFactory(stdin='secret\n',
-                                 stdout=tests.StringIOWrapper(),
-                                 stderr=tests.StringIOWrapper())
+        ui = self.make_test_ui_factory('secret\n')
         pb = ui.nested_progress_bar()
         try:
             self.assertEqual('secret',
@@ -83,9 +96,7 @@
         We can't predict what encoding users will have for stdin, so we force
         it to utf8 to test that we transport the password correctly.
         """
-        ui = tests.TestUIFactory(stdin=u'baz\u1234'.encode('utf8'),
-                                 stdout=tests.StringIOWrapper(),
-                                 stderr=tests.StringIOWrapper())
+        ui = self.make_test_ui_factory(u'baz\u1234'.encode('utf8'))
         ui.stderr.encoding = ui.stdout.encoding = ui.stdin.encoding = 'utf8'
         pb = ui.nested_progress_bar()
         try:
@@ -437,6 +448,39 @@
         self.assertIsNone('off', av)
 
 
+class TestConfirmationUserInterfacePolicy(tests.TestCase):
+
+    def test_confirm_action_default(self):
+        base_ui = _mod_ui.NoninteractiveUIFactory()
+        for answer in [True, False]:
+            self.assertEquals(
+                _mod_ui.ConfirmationUserInterfacePolicy(base_ui, answer, {})
+                .confirm_action("Do something?",
+                    "bzrlib.tests.do_something", {}),
+                answer)
+
+    def test_confirm_action_specific(self):
+        base_ui = _mod_ui.NoninteractiveUIFactory()
+        for default_answer in [True, False]:
+            for specific_answer in [True, False]:
+                for conf_id in ['given_id', 'other_id']:
+                    wrapper = _mod_ui.ConfirmationUserInterfacePolicy(
+                        base_ui, default_answer, dict(given_id=specific_answer))
+                    result = wrapper.confirm_action("Do something?", conf_id, {})
+                    if conf_id == 'given_id':
+                        self.assertEquals(result, specific_answer)
+                    else:
+                        self.assertEquals(result, default_answer)
+
+    def test_repr(self):
+        base_ui = _mod_ui.NoninteractiveUIFactory()
+        wrapper = _mod_ui.ConfirmationUserInterfacePolicy(
+            base_ui, True, dict(a=2))
+        self.assertThat(repr(wrapper),
+            Equals("ConfirmationUserInterfacePolicy("
+                "NoninteractiveUIFactory(), True, {'a': 2})"))
+
+
 class TestProgressRecordingUI(tests.TestCase):
     """Test test-oriented UIFactory that records progress updates"""
 

=== modified file 'bzrlib/ui/__init__.py'
--- a/bzrlib/ui/__init__.py	2010-06-25 20:34:05 +0000
+++ b/bzrlib/ui/__init__.py	2010-09-14 06:46:18 +0000
@@ -100,6 +100,42 @@
     return val
 
 
+class ConfirmationUserInterfacePolicy(object):
+    """Wrapper for a UIFactory that allows or denies all confirmed actions."""
+
+    def __init__(self, wrapped_ui, default_answer, specific_answers):
+        """Generate a proxy UI that does no confirmations.
+
+        :param wrapped_ui: Underlying UIFactory.
+        :param default_answer: Bool for whether requests for
+            confirmation from the user should be noninteractively accepted or
+            denied.
+        :param specific_answers: Map from confirmation_id to bool answer.
+        """
+        self.wrapped_ui = wrapped_ui
+        self.default_answer = default_answer
+        self.specific_answers = specific_answers
+
+    def __getattr__(self, name):
+        return getattr(self.wrapped_ui, name)
+
+    def __repr__(self):
+        return '%s(%r, %r, %r)' % (
+            self.__class__.__name__,
+            self.wrapped_ui,
+            self.default_answer, 
+            self.specific_answers)
+
+    def confirm_action(self, prompt, confirmation_id, prompt_kwargs):
+        if confirmation_id in self.specific_answers:
+            return self.specific_answers[confirmation_id]
+        elif self.default_answer is not None:
+            return self.default_answer
+        else:
+            return self.wrapped_ui.confirm_action(
+                prompt, confirmation_id, prompt_kwargs)
+
+
 class UIFactory(object):
     """UI abstraction.
 
@@ -151,6 +187,24 @@
         """
         self._quiet = state
 
+    def confirm_action(self, prompt, confirmation_id, prompt_kwargs):
+        """Seek user confirmation for an action.
+
+        If the UI is noninteractive, or the user does not want to be asked
+        about this action, True is returned, indicating bzr should just
+        proceed.
+
+        The confirmation id allows the user to configure certain actions to
+        always be confirmed or always denied, and for UIs to specialize the
+        display of particular confirmations.
+
+        :param prompt: Suggested text to display to the user.
+        :param prompt_kwargs: A dictionary of arguments that can be
+            string-interpolated into the prompt.
+        :param confirmation_id: Unique string identifier for the confirmation.
+        """
+        return self.get_boolean(prompt % prompt_kwargs)
+
     def get_password(self, prompt='', **kwargs):
         """Prompt the user for a password.
 
@@ -377,7 +431,17 @@
                 "without an upgrade path.\n" % (inter.target._format,))
 
 
-class SilentUIFactory(UIFactory):
+class NoninteractiveUIFactory(UIFactory):
+    """Base class for UIs with no user."""
+
+    def confirm_action(self, prompt, confirmation_id, prompt_kwargs):
+        return True
+
+    def __repr__(self):
+        return '%s()' % (self.__class__.__name__, )
+
+
+class SilentUIFactory(NoninteractiveUIFactory):
     """A UI Factory which never prints anything.
 
     This is the default UI, if another one is never registered by a program
@@ -418,6 +482,9 @@
     def __repr__(self):
         return "%s(%r)" % (self.__class__.__name__, self.responses)
 
+    def confirm_action(self, prompt, confirmation_id, args):
+        return self.get_boolean(prompt % args)
+
     def get_boolean(self, prompt):
         return self.responses.pop(0)
 

=== modified file 'doc/developers/ui.txt'
--- a/doc/developers/ui.txt	2010-09-03 09:14:12 +0000
+++ b/doc/developers/ui.txt	2010-09-14 06:46:18 +0000
@@ -1,6 +1,6 @@
-==========================================
-Bazaar Developer Guide to User Interaction
-==========================================
+=========================
+Interacting with the user
+=========================
 
 Getting Input
 =============
@@ -24,6 +24,40 @@
 used for programmer convenience, but not performing unpredictably in the
 presence of different locales.
 
+Confirmation
+============
+
+There are some operations, such as uncommitting, or breaking a lock, where
+bzr may want to get confirmation from the user before proceeding.
+However in some circumstances bzr should just go ahead without asking: if
+it's being used from a noninteractive program, or if the user's asked to
+never be asked for this particular confirmation or for any confirmations
+at all.
+
+We provide a special UIFactory method `confirm_action` to do this.  It
+takes a `confirmation_id` parameter that acts as a symbolic name for the
+type of confirmation, so the user can configure them off.  (This is not
+implemented at present.)  GUIs can have a "don't ask me again" option
+keyed by the confirmation id.
+
+Confirmation ids look like Python paths to the logical code that should
+use them.  (Because code may move or the check may for implementation
+reasons be done elsewhere, they need not perfectly correspond to the place
+they're used, and they should stay stable even when the code moves.)
+
+``bzrlib.builtins.uncommit``
+    Before the ``uncommit`` command actually changes the branch.
+
+``bzrlib.lockdir.break``
+    Before breaking a lock.
+
+``bzrlib.msgeditor.unchanged``
+    Proceed even though the user made no changes to the template message.
+
+Interactive confirmations can be overridden by using a
+`ConfirmationUserInterfacePolicy` decorator as the default
+ui_factory.
+
 
 Writing Output
 ==============




More information about the bazaar-commits mailing list