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

Proposed by Vincent Ladeuil
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
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 Approve
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.
Revision history for this message
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
Revision history for this message
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
=== modified file 'bzrlib/config.py'
--- bzrlib/config.py 2012-09-18 16:38:02 +0000
+++ bzrlib/config.py 2012-09-19 07:15:22 +0000
@@ -2135,7 +2135,7 @@
21352135
2136class Base64CredentialStore(CredentialStore):2136class Base64CredentialStore(CredentialStore):
2137 __doc__ = """Base64 credential store for the authentication.conf file"""2137 __doc__ = """Base64 credential store for the authentication.conf file"""
2138 2138
2139 def decode_password(self, credentials):2139 def decode_password(self, credentials):
2140 """See CredentialStore.decode_password."""2140 """See CredentialStore.decode_password."""
2141 # GZ 2012-07-28: Will raise binascii.Error if password is not base64,2141 # GZ 2012-07-28: Will raise binascii.Error if password is not base64,
@@ -2161,7 +2161,8 @@
2161 for those under repositories.2161 for those under repositories.
2162 """2162 """
2163 if self._config is None:2163 if self._config is None:
2164 raise errors.BzrError("Cannot set configuration in %s" % self._bzrdir)2164 raise errors.BzrError("Cannot set configuration in %s"
2165 % self._bzrdir)
2165 if value is None:2166 if value is None:
2166 self._config.set_option('', 'default_stack_on')2167 self._config.set_option('', 'default_stack_on')
2167 else:2168 else:
@@ -2473,8 +2474,8 @@
2473 return float(unicode_str)2474 return float(unicode_str)
24742475
24752476
2476# Use a an empty dict to initialize an empty configobj avoiding all2477# Use an empty dict to initialize an empty configobj avoiding all parsing and
2477# parsing and encoding checks2478# encoding checks
2478_list_converter_config = configobj.ConfigObj(2479_list_converter_config = configobj.ConfigObj(
2479 {}, encoding='utf-8', list_values=True, interpolation=False)2480 {}, encoding='utf-8', list_values=True, interpolation=False)
24802481
@@ -2802,6 +2803,8 @@
28022803
2803Each function takes branch, rev_id as parameters.2804Each function takes branch, rev_id as parameters.
2804'''))2805'''))
2806option_registry.register_lazy('progress_bar', 'bzrlib.ui.text',
2807 'opt_progress_bar')
2805option_registry.register(2808option_registry.register(
2806 Option('public_branch',2809 Option('public_branch',
2807 default=None,2810 default=None,
28082811
=== modified file 'bzrlib/tests/blackbox/test_debug.py'
--- bzrlib/tests/blackbox/test_debug.py 2011-02-04 22:25:59 +0000
+++ bzrlib/tests/blackbox/test_debug.py 2012-09-19 07:15:22 +0000
@@ -52,9 +52,8 @@
52 # I would like to avoid run_bzr_subprocess here, but we need it to be52 # I would like to avoid run_bzr_subprocess here, but we need it to be
53 # connected to a real TextUIFactory. The NullProgressView always53 # connected to a real TextUIFactory. The NullProgressView always
54 # ignores transport activity.54 # ignores transport activity.
55 env = {'BZR_PROGRESS_BAR': 'text'}55 out, err = self.run_bzr_subprocess(
56 out, err = self.run_bzr_subprocess('branch -Dbytes %s/tree target'56 'branch -Dbytes -Oprogress_bar=text %s/tree target'
57 % (remote_trans.base,),57 % (remote_trans.base,))
58 env_changes=env)
59 self.assertContainsRe(err, 'Branched 1 revision')58 self.assertContainsRe(err, 'Branched 1 revision')
60 self.assertContainsRe(err, 'Transferred:.*kB')59 self.assertContainsRe(err, 'Transferred:.*kB')
6160
=== modified file 'bzrlib/tests/blackbox/test_diff.py'
--- bzrlib/tests/blackbox/test_diff.py 2012-01-05 13:02:31 +0000
+++ bzrlib/tests/blackbox/test_diff.py 2012-09-19 07:15:22 +0000
@@ -388,10 +388,10 @@
388 # subprocess.py that we had to workaround).388 # subprocess.py that we had to workaround).
389 # However, if 'diff' may not be available389 # However, if 'diff' may not be available
390 self.make_example_branch()390 self.make_example_branch()
391 self.overrideEnv('BZR_PROGRESS_BAR', 'none')391 out, err = self.run_bzr_subprocess(
392 out, err = self.run_bzr_subprocess('diff -r 1 --diff-options -ub',392 'diff -Oprogress_bar=none -r 1 --diff-options -ub',
393 universal_newlines=True,393 universal_newlines=True,
394 retcode=None)394 retcode=None)
395 if 'Diff is not installed on this machine' in err:395 if 'Diff is not installed on this machine' in err:
396 raise tests.TestSkipped("No external 'diff' is available")396 raise tests.TestSkipped("No external 'diff' is available")
397 self.assertEqual('', err)397 self.assertEqual('', err)
398398
=== modified file 'bzrlib/tests/blackbox/test_locale.py'
--- bzrlib/tests/blackbox/test_locale.py 2011-05-15 14:35:24 +0000
+++ bzrlib/tests/blackbox/test_locale.py 2012-09-19 07:15:22 +0000
@@ -41,15 +41,21 @@
41 timestamp=1156451297.96, timezone=0)41 timestamp=1156451297.96, timezone=0)
42 self.tree = tree42 self.tree = tree
4343
44 def run_log_quiet_long(self, args, env_changes={}):
45 cmd = ['--no-aliases', '--no-plugins', '-Oprogress_bar=none',
46 'log', '-q', '--log-format=long']
47 cmd.extend(args)
48 return self.run_bzr_subprocess(cmd, env_changes=env_changes)
49
44 def test_log_C(self):50 def test_log_C(self):
45 self.disable_missing_extensions_warning()51 self.disable_missing_extensions_warning()
46 # C is not necessarily the default locale, so set both LANG and LC_ALL52 out, err = self.run_log_quiet_long(
47 # explicitly because LC_ALL is preferred on (some?) Linux systems but53 ['tree'],
48 # only LANG is respected on Windows.54 # C is not necessarily the default locale, so set both LANG and
49 out, err = self.run_bzr_subprocess(55 # LC_ALL explicitly because LC_ALL is preferred on (some?) Linux
50 '--no-aliases --no-plugins log -q --log-format=long tree',56 # systems but only LANG is respected on Windows.
51 env_changes={'LANG': 'C', 'BZR_PROGRESS_BAR':'none',57 env_changes={'LANG': 'C', 'LC_ALL': 'C', 'LC_CTYPE':None,
52 'LC_ALL': 'C', 'LC_CTYPE':None, 'LANGUAGE':None})58 'LANGUAGE':None})
53 self.assertEqual('', err)59 self.assertEqual('', err)
54 self.assertEqualDiff("""\60 self.assertEqualDiff("""\
55------------------------------------------------------------61------------------------------------------------------------
@@ -62,10 +68,10 @@
62""", out)68""", out)
6369
64 def test_log_BOGUS(self):70 def test_log_BOGUS(self):
65 out, err = self.run_bzr_subprocess(71 out, err = self.run_log_quiet_long(
66 '--no-aliases --no-plugins log -q --log-format=long tree',72 ['tree'],
67 env_changes={'LANG':'BOGUS', 'BZR_PROGRESS_BAR':'none',73 env_changes={'LANG':'BOGUS', 'LC_ALL':None, 'LC_CTYPE':None,
68 'LC_ALL':None, 'LC_CTYPE':None, 'LANGUAGE':None})74 'LANGUAGE':None})
69 self.assertStartsWith(err, 'bzr: warning: unsupported locale setting')75 self.assertStartsWith(err, 'bzr: warning: unsupported locale setting')
70 self.assertEqualDiff("""\76 self.assertEqualDiff("""\
71------------------------------------------------------------77------------------------------------------------------------
7278
=== modified file 'bzrlib/ui/text.py'
--- bzrlib/ui/text.py 2012-04-30 10:44:04 +0000
+++ bzrlib/ui/text.py 2012-09-19 07:15:22 +0000
@@ -31,6 +31,7 @@
31import warnings31import warnings
3232
33from bzrlib import (33from bzrlib import (
34 config,
34 debug,35 debug,
35 progress,36 progress,
36 osutils,37 osutils,
@@ -155,8 +156,14 @@
155 return index156 return index
156157
157158
159opt_progress_bar = config.Option(
160 'progress_bar', help='Progress bar type.',
161 default_from_env=['BZR_PROGRESS_BAR'], default=None,
162 invalid='error')
163
164
158class TextUIFactory(UIFactory):165class TextUIFactory(UIFactory):
159 """A UI factory for Text user interefaces."""166 """A UI factory for Text user interfaces."""
160167
161 def __init__(self,168 def __init__(self,
162 stdin=None,169 stdin=None,
@@ -279,14 +286,14 @@
279 # do that. otherwise, guess based on $TERM and tty presence.286 # do that. otherwise, guess based on $TERM and tty presence.
280 if self.is_quiet():287 if self.is_quiet():
281 return NullProgressView()288 return NullProgressView()
282 elif os.environ.get('BZR_PROGRESS_BAR') == 'text':289 pb_type = config.GlobalStack().get('progress_bar')
283 return TextProgressView(self.stderr)290 if pb_type == 'none': # Explicit requirement
284 elif os.environ.get('BZR_PROGRESS_BAR') == 'none':291 return NullProgressView()
285 return NullProgressView()292 if (pb_type == 'text' # Explicit requirement
286 elif progress._supports_progress(self.stderr):293 or progress._supports_progress(self.stderr)): # Guess
287 return TextProgressView(self.stderr)294 return TextProgressView(self.stderr)
288 else:295 # No explicit requirement and no successful guess
289 return NullProgressView()296 return NullProgressView()
290297
291 def _make_output_stream_explicit(self, encoding, encoding_type):298 def _make_output_stream_explicit(self, encoding, encoding_type):
292 if encoding_type == 'exact':299 if encoding_type == 'exact':
293300
=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt 2012-09-18 16:38:02 +0000
+++ doc/en/release-notes/bzr-2.6.txt 2012-09-19 07:15:22 +0000
@@ -44,6 +44,10 @@
44Bug Fixes44Bug Fixes
45*********45*********
4646
47* Add a ``progress_bar`` configuration option defaulting to
48 ``BZR_PROGRESS_BAR``. This can be set in ``bazaar.conf`` or specified from
49 the command line with ``-Oprogress_bar=text``. (Vincent Ladeuil, #388725)
50
47* Fixed a bug where the entire contents of ``/etc/mailname`` is read in.51* Fixed a bug where the entire contents of ``/etc/mailname`` is read in.
48 We only want to read in the first line so that comments could be added52 We only want to read in the first line so that comments could be added
49 and would be ignored.53 and would be ignored.