Merge lp:~vila/bzr/1235099-illegal-option-names into lp:bzr

Proposed by Vincent Ladeuil on 2016-01-21
Status: Merged
Approved by: Richard Wilbur on 2016-01-21
Approved revision: 6591
Merged at revision: 6610
Proposed branch: lp:~vila/bzr/1235099-illegal-option-names
Merge into: lp:bzr
Diff against target: 59 lines (+11/-5)
3 files modified
bzrlib/config.py (+2/-2)
bzrlib/tests/test_config.py (+6/-0)
doc/en/release-notes/bzr-2.7.txt (+3/-3)
To merge this branch: bzr merge lp:~vila/bzr/1235099-illegal-option-names
Reviewer Review Type Date Requested Status
Richard Wilbur 2016-01-21 Approve on 2016-01-21
Review via email: mp+283430@code.launchpad.net

Commit message

Revise legal option names to be less drastic.

Description of the change

This revisit the previous patch to allow hyphens in option names again.

The previous patch said:

> This proposal doesn't change which option names are legal (only invalid references could be used before and led to failures) but give better error messages.

This was overly restrictive and broke bzr-svn use of 'guessed-layout' for no good reasons.

To post a comment you must log in.
Richard Wilbur (richard-wilbur) wrote :

Thanks, Vincent, for adapting the option parser to allow bzr-svn's use of hyphens.
+2

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
=== modified file 'bzrlib/config.py'
--- bzrlib/config.py 2014-06-19 09:42:08 +0000
+++ bzrlib/config.py 2016-01-21 11:45:35 +0000
@@ -1,4 +1,4 @@
1# Copyright (C) 2005-2012 Canonical Ltd1# Copyright (C) 2005-2014, 2016 Canonical Ltd
2# Authors: Robert Collins <robert.collins@canonical.com>2# Authors: Robert Collins <robert.collins@canonical.com>
3# and others3# and others
4#4#
@@ -2550,7 +2550,7 @@
2550 return "".join(ret)2550 return "".join(ret)
25512551
25522552
2553_option_ref_re = lazy_regex.lazy_compile('({[^\d\W](?:\.\w|\w)*})')2553_option_ref_re = lazy_regex.lazy_compile('({[^\d\W](?:\.\w|-\w|\w)*})')
2554"""Describes an expandable option reference.2554"""Describes an expandable option reference.
25552555
2556We want to match the most embedded reference first.2556We want to match the most embedded reference first.
25572557
=== modified file 'bzrlib/tests/test_config.py'
--- bzrlib/tests/test_config.py 2016-01-14 14:10:33 +0000
+++ bzrlib/tests/test_config.py 2016-01-21 11:45:35 +0000
@@ -2237,6 +2237,8 @@
2237 self.assertTrue(self.is_valid('__bar__'))2237 self.assertTrue(self.is_valid('__bar__'))
2238 self.assertTrue(self.is_valid('a_'))2238 self.assertTrue(self.is_valid('a_'))
2239 self.assertTrue(self.is_valid('a1'))2239 self.assertTrue(self.is_valid('a1'))
2240 # Don't break bzr-svn for no good reason
2241 self.assertTrue(self.is_valid('guessed-layout'))
22402242
2241 def test_invalid_names(self):2243 def test_invalid_names(self):
2242 self.assertFalse(self.is_valid(' foo'))2244 self.assertFalse(self.is_valid(' foo'))
@@ -2250,6 +2252,10 @@
2250 self.assertFalse(self.is_valid('{}'))2252 self.assertFalse(self.is_valid('{}'))
2251 self.assertFalse(self.is_valid('{a}'))2253 self.assertFalse(self.is_valid('{a}'))
2252 self.assertFalse(self.is_valid('a\n'))2254 self.assertFalse(self.is_valid('a\n'))
2255 self.assertFalse(self.is_valid('-'))
2256 self.assertFalse(self.is_valid('-a'))
2257 self.assertFalse(self.is_valid('a-'))
2258 self.assertFalse(self.is_valid('a--a'))
22532259
2254 def assertSingleGroup(self, reference):2260 def assertSingleGroup(self, reference):
2255 # the regexp is used with split and as such should match the reference2261 # the regexp is used with split and as such should match the reference
22562262
=== modified file 'doc/en/release-notes/bzr-2.7.txt'
--- doc/en/release-notes/bzr-2.7.txt 2016-01-14 14:10:33 +0000
+++ doc/en/release-notes/bzr-2.7.txt 2016-01-21 11:45:35 +0000
@@ -39,9 +39,9 @@
39* 'acceptable_keys' from 'bazaar.conf' is now properly handled.39* 'acceptable_keys' from 'bazaar.conf' is now properly handled.
40 (Vincent Ladeuil, #1249732)40 (Vincent Ladeuil, #1249732)
4141
42* Option names are now checked to be valid [dotted] python identifiers. Also42* Option names are now checked to be valid identifiers (including embedded
43 ignore invalid references (i.e. using invalid option names) while43 dots or hyphens). Also ignore invalid references (i.e. using invalid
44 expanding option values. (Vincent Ladeuil, #1235099)44 option names) while expanding option values. (Vincent Ladeuil, #1235099)
4545
46* Fix pyrex version checking to be more robust.46* Fix pyrex version checking to be more robust.
47 (Andrew Starr-Bochicchio, #1030521 )47 (Andrew Starr-Bochicchio, #1030521 )