Merge lp:~vila/bzr/388725-progress-bar-config-option into lp:bzr

Proposed by Vincent Ladeuil on 2012-09-19
Status: Merged
Approved by: John A Meinel on 2012-09-19
Approved revision: 6562
Merged at revision: 6564
Proposed branch: lp:~vila/bzr/388725-progress-bar-config-option
Merge into: lp:bzr
Diff against target: 192 lines (+51/-32)
6 files modified
bzrlib/config.py (+7/-4)
bzrlib/tests/blackbox/test_debug.py (+3/-4)
bzrlib/tests/blackbox/test_diff.py (+4/-4)
bzrlib/tests/blackbox/test_locale.py (+17/-11)
bzrlib/ui/text.py (+16/-9)
doc/en/release-notes/bzr-2.6.txt (+4/-0)
To merge this branch: bzr merge lp:~vila/bzr/388725-progress-bar-config-option
Reviewer Review Type Date Requested Status
John A Meinel 2012-09-19 Approve on 2012-09-19
Review via email: mp+125114@code.launchpad.net

Commit message

Add a config option for the progress bar type.

Description of the change

Add a config option for the progress bar type.

I fixed this bug with a simple grep, splitting tests between those that use
'-Oprogress_bar=[none|text]' and those that still use the env bar and was
pleasantly surprised to see this was so trivial do decide... it demonstrates
(for those that weren't convinced, slightly including myself ;) that there
is indeed a good use case to allow both the env var and the config option.

To post a comment you must log in.
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 9/19/2012 11:16 AM, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging
> lp:~vila/bzr/388725-progress-bar-config-option into lp:bzr.
>
> Requested reviews: bzr-core (bzr-core) Related bugs: Bug #388725
> in Duplicity: "Report when duplicity is 'done' with restoring a
> file" https://bugs.launchpad.net/duplicity/+bug/388725
>
> For more details, see:
> https://code.launchpad.net/~vila/bzr/388725-progress-bar-config-option/+merge/125114
>
>
>
Add a config option for the progress bar type.
>
> I fixed this bug with a simple grep, splitting tests between those
> that use '-Oprogress_bar=[none|text]' and those that still use the
> env bar and was pleasantly surprised to see this was so trivial do
> decide... it demonstrates (for those that weren't convinced,
> slightly including myself ;) that there is indeed a good use case
> to allow both the env var and the config option.
>

 merge: approve

Looks good.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlBZcooACgkQJdeBCYSNAAND4ACgiLVx+BUb82iYjIgoNNpbrF5I
9UIAnRG5km6BainfXIyCkz1G1TlVOFhG
=3puI
-----END PGP SIGNATURE-----

review: Approve
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/config.py'
2--- bzrlib/config.py 2012-09-18 16:38:02 +0000
3+++ bzrlib/config.py 2012-09-19 07:15:22 +0000
4@@ -2135,7 +2135,7 @@
5
6 class Base64CredentialStore(CredentialStore):
7 __doc__ = """Base64 credential store for the authentication.conf file"""
8-
9+
10 def decode_password(self, credentials):
11 """See CredentialStore.decode_password."""
12 # GZ 2012-07-28: Will raise binascii.Error if password is not base64,
13@@ -2161,7 +2161,8 @@
14 for those under repositories.
15 """
16 if self._config is None:
17- raise errors.BzrError("Cannot set configuration in %s" % self._bzrdir)
18+ raise errors.BzrError("Cannot set configuration in %s"
19+ % self._bzrdir)
20 if value is None:
21 self._config.set_option('', 'default_stack_on')
22 else:
23@@ -2473,8 +2474,8 @@
24 return float(unicode_str)
25
26
27-# Use a an empty dict to initialize an empty configobj avoiding all
28-# parsing and encoding checks
29+# Use an empty dict to initialize an empty configobj avoiding all parsing and
30+# encoding checks
31 _list_converter_config = configobj.ConfigObj(
32 {}, encoding='utf-8', list_values=True, interpolation=False)
33
34@@ -2802,6 +2803,8 @@
35
36 Each function takes branch, rev_id as parameters.
37 '''))
38+option_registry.register_lazy('progress_bar', 'bzrlib.ui.text',
39+ 'opt_progress_bar')
40 option_registry.register(
41 Option('public_branch',
42 default=None,
43
44=== modified file 'bzrlib/tests/blackbox/test_debug.py'
45--- bzrlib/tests/blackbox/test_debug.py 2011-02-04 22:25:59 +0000
46+++ bzrlib/tests/blackbox/test_debug.py 2012-09-19 07:15:22 +0000
47@@ -52,9 +52,8 @@
48 # I would like to avoid run_bzr_subprocess here, but we need it to be
49 # connected to a real TextUIFactory. The NullProgressView always
50 # ignores transport activity.
51- env = {'BZR_PROGRESS_BAR': 'text'}
52- out, err = self.run_bzr_subprocess('branch -Dbytes %s/tree target'
53- % (remote_trans.base,),
54- env_changes=env)
55+ out, err = self.run_bzr_subprocess(
56+ 'branch -Dbytes -Oprogress_bar=text %s/tree target'
57+ % (remote_trans.base,))
58 self.assertContainsRe(err, 'Branched 1 revision')
59 self.assertContainsRe(err, 'Transferred:.*kB')
60
61=== modified file 'bzrlib/tests/blackbox/test_diff.py'
62--- bzrlib/tests/blackbox/test_diff.py 2012-01-05 13:02:31 +0000
63+++ bzrlib/tests/blackbox/test_diff.py 2012-09-19 07:15:22 +0000
64@@ -388,10 +388,10 @@
65 # subprocess.py that we had to workaround).
66 # However, if 'diff' may not be available
67 self.make_example_branch()
68- self.overrideEnv('BZR_PROGRESS_BAR', 'none')
69- out, err = self.run_bzr_subprocess('diff -r 1 --diff-options -ub',
70- universal_newlines=True,
71- retcode=None)
72+ out, err = self.run_bzr_subprocess(
73+ 'diff -Oprogress_bar=none -r 1 --diff-options -ub',
74+ universal_newlines=True,
75+ retcode=None)
76 if 'Diff is not installed on this machine' in err:
77 raise tests.TestSkipped("No external 'diff' is available")
78 self.assertEqual('', err)
79
80=== modified file 'bzrlib/tests/blackbox/test_locale.py'
81--- bzrlib/tests/blackbox/test_locale.py 2011-05-15 14:35:24 +0000
82+++ bzrlib/tests/blackbox/test_locale.py 2012-09-19 07:15:22 +0000
83@@ -41,15 +41,21 @@
84 timestamp=1156451297.96, timezone=0)
85 self.tree = tree
86
87+ def run_log_quiet_long(self, args, env_changes={}):
88+ cmd = ['--no-aliases', '--no-plugins', '-Oprogress_bar=none',
89+ 'log', '-q', '--log-format=long']
90+ cmd.extend(args)
91+ return self.run_bzr_subprocess(cmd, env_changes=env_changes)
92+
93 def test_log_C(self):
94 self.disable_missing_extensions_warning()
95- # C is not necessarily the default locale, so set both LANG and LC_ALL
96- # explicitly because LC_ALL is preferred on (some?) Linux systems but
97- # only LANG is respected on Windows.
98- out, err = self.run_bzr_subprocess(
99- '--no-aliases --no-plugins log -q --log-format=long tree',
100- env_changes={'LANG': 'C', 'BZR_PROGRESS_BAR':'none',
101- 'LC_ALL': 'C', 'LC_CTYPE':None, 'LANGUAGE':None})
102+ out, err = self.run_log_quiet_long(
103+ ['tree'],
104+ # C is not necessarily the default locale, so set both LANG and
105+ # LC_ALL explicitly because LC_ALL is preferred on (some?) Linux
106+ # systems but only LANG is respected on Windows.
107+ env_changes={'LANG': 'C', 'LC_ALL': 'C', 'LC_CTYPE':None,
108+ 'LANGUAGE':None})
109 self.assertEqual('', err)
110 self.assertEqualDiff("""\
111 ------------------------------------------------------------
112@@ -62,10 +68,10 @@
113 """, out)
114
115 def test_log_BOGUS(self):
116- out, err = self.run_bzr_subprocess(
117- '--no-aliases --no-plugins log -q --log-format=long tree',
118- env_changes={'LANG':'BOGUS', 'BZR_PROGRESS_BAR':'none',
119- 'LC_ALL':None, 'LC_CTYPE':None, 'LANGUAGE':None})
120+ out, err = self.run_log_quiet_long(
121+ ['tree'],
122+ env_changes={'LANG':'BOGUS', 'LC_ALL':None, 'LC_CTYPE':None,
123+ 'LANGUAGE':None})
124 self.assertStartsWith(err, 'bzr: warning: unsupported locale setting')
125 self.assertEqualDiff("""\
126 ------------------------------------------------------------
127
128=== modified file 'bzrlib/ui/text.py'
129--- bzrlib/ui/text.py 2012-04-30 10:44:04 +0000
130+++ bzrlib/ui/text.py 2012-09-19 07:15:22 +0000
131@@ -31,6 +31,7 @@
132 import warnings
133
134 from bzrlib import (
135+ config,
136 debug,
137 progress,
138 osutils,
139@@ -155,8 +156,14 @@
140 return index
141
142
143+opt_progress_bar = config.Option(
144+ 'progress_bar', help='Progress bar type.',
145+ default_from_env=['BZR_PROGRESS_BAR'], default=None,
146+ invalid='error')
147+
148+
149 class TextUIFactory(UIFactory):
150- """A UI factory for Text user interefaces."""
151+ """A UI factory for Text user interfaces."""
152
153 def __init__(self,
154 stdin=None,
155@@ -279,14 +286,14 @@
156 # do that. otherwise, guess based on $TERM and tty presence.
157 if self.is_quiet():
158 return NullProgressView()
159- elif os.environ.get('BZR_PROGRESS_BAR') == 'text':
160- return TextProgressView(self.stderr)
161- elif os.environ.get('BZR_PROGRESS_BAR') == 'none':
162- return NullProgressView()
163- elif progress._supports_progress(self.stderr):
164- return TextProgressView(self.stderr)
165- else:
166- return NullProgressView()
167+ pb_type = config.GlobalStack().get('progress_bar')
168+ if pb_type == 'none': # Explicit requirement
169+ return NullProgressView()
170+ if (pb_type == 'text' # Explicit requirement
171+ or progress._supports_progress(self.stderr)): # Guess
172+ return TextProgressView(self.stderr)
173+ # No explicit requirement and no successful guess
174+ return NullProgressView()
175
176 def _make_output_stream_explicit(self, encoding, encoding_type):
177 if encoding_type == 'exact':
178
179=== modified file 'doc/en/release-notes/bzr-2.6.txt'
180--- doc/en/release-notes/bzr-2.6.txt 2012-09-18 16:38:02 +0000
181+++ doc/en/release-notes/bzr-2.6.txt 2012-09-19 07:15:22 +0000
182@@ -44,6 +44,10 @@
183 Bug Fixes
184 *********
185
186+* Add a ``progress_bar`` configuration option defaulting to
187+ ``BZR_PROGRESS_BAR``. This can be set in ``bazaar.conf`` or specified from
188+ the command line with ``-Oprogress_bar=text``. (Vincent Ladeuil, #388725)
189+
190 * Fixed a bug where the entire contents of ``/etc/mailname`` is read in.
191 We only want to read in the first line so that comments could be added
192 and would be ignored.