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

[Vincent Ladeuil]

Revise legal option names to be less drastic.

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.

[Richard Wilbur (richard-wilbur)]

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

review: Approve

[Vincent Ladeuil (vila)]

sent to pqm by email

=== 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 @@
-# Copyright (C) 2005-2012 Canonical Ltd
+# Copyright (C) 2005-2014, 2016 Canonical Ltd
 #   Authors: Robert Collins <[e-mail]>
 #            and others
 #
@@ -2550,7 +2550,7 @@
         return "".join(ret)
 
 
-_option_ref_re = lazy_regex.lazy_compile('({[^\d\W](?:\.\w|\w)*})')
+_option_ref_re = lazy_regex.lazy_compile('({[^\d\W](?:\.\w|-\w|\w)*})')
 """Describes an expandable option reference.
 
 We want to match the most embedded reference first.

=== 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 @@
         self.assertTrue(self.is_valid('__bar__'))
         self.assertTrue(self.is_valid('a_'))
         self.assertTrue(self.is_valid('a1'))
+        # Don't break bzr-svn for no good reason
+        self.assertTrue(self.is_valid('guessed-layout'))
 
     def test_invalid_names(self):
         self.assertFalse(self.is_valid(' foo'))
@@ -2250,6 +2252,10 @@
         self.assertFalse(self.is_valid('{}'))
         self.assertFalse(self.is_valid('{a}'))
         self.assertFalse(self.is_valid('a\n'))
+        self.assertFalse(self.is_valid('-'))
+        self.assertFalse(self.is_valid('-a'))
+        self.assertFalse(self.is_valid('a-'))
+        self.assertFalse(self.is_valid('a--a'))
 
     def assertSingleGroup(self, reference):
         # the regexp is used with split and as such should match the reference

=== 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 @@
 * 'acceptable_keys' from 'bazaar.conf' is now properly handled.
   (Vincent Ladeuil, #1249732)
 
-* Option names are now checked to be valid [dotted] python identifiers. Also
-  ignore invalid references (i.e. using invalid option names) while
-  expanding option values. (Vincent Ladeuil, #1235099)
+* Option names are now checked to be valid identifiers (including embedded
+  dots or hyphens). Also ignore invalid references (i.e. using invalid
+  option names) while expanding option values. (Vincent Ladeuil, #1235099)
 
 * Fix pyrex version checking to be more robust.
   (Andrew Starr-Bochicchio, #1030521 )