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
1=== modified file 'bzrlib/config.py'
2--- bzrlib/config.py 2014-06-19 09:42:08 +0000
3+++ bzrlib/config.py 2016-01-21 11:45:35 +0000
4@@ -1,4 +1,4 @@
5-# Copyright (C) 2005-2012 Canonical Ltd
6+# Copyright (C) 2005-2014, 2016 Canonical Ltd
7 # Authors: Robert Collins <robert.collins@canonical.com>
8 # and others
9 #
10@@ -2550,7 +2550,7 @@
11 return "".join(ret)
12
13
14-_option_ref_re = lazy_regex.lazy_compile('({[^\d\W](?:\.\w|\w)*})')
15+_option_ref_re = lazy_regex.lazy_compile('({[^\d\W](?:\.\w|-\w|\w)*})')
16 """Describes an expandable option reference.
17
18 We want to match the most embedded reference first.
19
20=== modified file 'bzrlib/tests/test_config.py'
21--- bzrlib/tests/test_config.py 2016-01-14 14:10:33 +0000
22+++ bzrlib/tests/test_config.py 2016-01-21 11:45:35 +0000
23@@ -2237,6 +2237,8 @@
24 self.assertTrue(self.is_valid('__bar__'))
25 self.assertTrue(self.is_valid('a_'))
26 self.assertTrue(self.is_valid('a1'))
27+ # Don't break bzr-svn for no good reason
28+ self.assertTrue(self.is_valid('guessed-layout'))
29
30 def test_invalid_names(self):
31 self.assertFalse(self.is_valid(' foo'))
32@@ -2250,6 +2252,10 @@
33 self.assertFalse(self.is_valid('{}'))
34 self.assertFalse(self.is_valid('{a}'))
35 self.assertFalse(self.is_valid('a\n'))
36+ self.assertFalse(self.is_valid('-'))
37+ self.assertFalse(self.is_valid('-a'))
38+ self.assertFalse(self.is_valid('a-'))
39+ self.assertFalse(self.is_valid('a--a'))
40
41 def assertSingleGroup(self, reference):
42 # the regexp is used with split and as such should match the reference
43
44=== modified file 'doc/en/release-notes/bzr-2.7.txt'
45--- doc/en/release-notes/bzr-2.7.txt 2016-01-14 14:10:33 +0000
46+++ doc/en/release-notes/bzr-2.7.txt 2016-01-21 11:45:35 +0000
47@@ -39,9 +39,9 @@
48 * 'acceptable_keys' from 'bazaar.conf' is now properly handled.
49 (Vincent Ladeuil, #1249732)
50
51-* Option names are now checked to be valid [dotted] python identifiers. Also
52- ignore invalid references (i.e. using invalid option names) while
53- expanding option values. (Vincent Ladeuil, #1235099)
54+* Option names are now checked to be valid identifiers (including embedded
55+ dots or hyphens). Also ignore invalid references (i.e. using invalid
56+ option names) while expanding option values. (Vincent Ladeuil, #1235099)
57
58 * Fix pyrex version checking to be more robust.
59 (Andrew Starr-Bochicchio, #1030521 )