Bug 1333564 - [manifestparser] Add support for inline comments, r?jmaher
Previously manifestparser only supported inline comments on conditional keys, such as
skip-if, fails-if, subsuite-if, etc. But on any other key name, inline comments weren't
supported. This was a bad situation because people saw these skip-if comments, and
naturally assumed inline comments worked everywhere. Then they were surprised when things
broke in mysterious ways.
This patch removes the special-casing for skip-if and friends and allows inline comments
everywhere. A caveat to this, is that ' #' is no longer a valid substring in a value.
MozReview-Commit-ID: Hr0BIwzTgaJ
--- a/testing/mozbase/docs/manifestparser.rst
+++ b/testing/mozbase/docs/manifestparser.rst
@@ -131,27 +131,35 @@ section, without adding possible include
.. code-block:: text
[parent:../manifest.ini]
Manifests are included relative to the directory of the manifest with
the `[include:]` directive unless they are absolute paths.
-By default you can use '#' as a comment character. Comments must start on
-a new line, inline comments are not supported.
+By default you can use '#' as a comment character. Comments can start a
+new line, or be inline.
.. code-block:: text
[roses.js]
# a valid comment
- color = red # not a valid comment
+ color = red # another valid comment
+
+Comment characters must be preceded by a space, or they will not be
+treated as comments.
-In the example above, the 'color' property will have the value 'red #
-not a valid comment'.
+.. code-block:: text
+
+ [test1.js]
+ url = https://foo.com/bar#baz
+
+The '#baz' anchor will not be stripped off, as it wasn't preceded by
+a space.
Special variable server-root
````````````````````````````
There is a special variable called `server-root` used for paths on the system.
This variable is deemed a path and will be expanded into its absolute form.
Because of the inheritant nature of the key/value pairs, if one requires a
system path, it must be absolute for it to be of any use in any included file.
--- a/testing/mozbase/manifestparser/manifestparser/ini.py
+++ b/testing/mozbase/manifestparser/manifestparser/ini.py
@@ -1,13 +1,14 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.
import os
+import sys
__all__ = ['read_ini', 'combine_fields']
def read_ini(fp, variables=None, default='DEFAULT', defaults_only=False,
comments=None, separators=None, strict=True, handle_defaults=True):
"""
read an .ini file and return a list of [(section, values)]
@@ -41,16 +42,33 @@ def read_ini(fp, variables=None, default
# reset key and value to avoid continuation lines
key = value = None
continue
# ignore comment lines
if any(stripped.startswith(c) for c in comments):
continue
+ # strip inline comments (borrowed from configparser)
+ comment_start = sys.maxsize
+ inline_prefixes = {p: -1 for p in comments}
+ while comment_start == sys.maxsize and inline_prefixes:
+ next_prefixes = {}
+ for prefix, index in inline_prefixes.items():
+ index = line.find(prefix, index+1)
+ if index == -1:
+ continue
+ next_prefixes[prefix] = index
+ if index == 0 or (index > 0 and line[index-1].isspace()):
+ comment_start = min(comment_start, index)
+ inline_prefixes = next_prefixes
+
+ if comment_start != sys.maxsize:
+ stripped = stripped[:comment_start].rstrip()
+
# check for a new section
if len(stripped) > 2 and stripped[0] == '[' and stripped[-1] == ']':
section = stripped[1:-1].strip()
key = value = None
# deal with DEFAULT section
if section.lower() == default.lower():
if strict:
@@ -133,11 +151,10 @@ def combine_fields(global_vars, local_va
}
final_mapping = global_vars.copy()
for field_name, value in local_vars.items():
if field_name not in field_patterns or field_name not in global_vars:
final_mapping[field_name] = value
continue
global_value = global_vars[field_name]
pattern = field_patterns[field_name]
- final_mapping[field_name] = pattern % (
- global_value.split('#')[0], value.split('#')[0])
+ final_mapping[field_name] = pattern % (global_value, value)
return final_mapping
--- a/testing/mozbase/manifestparser/tests/test_read_ini.py
+++ b/testing/mozbase/manifestparser/tests/test_read_ini.py
@@ -7,45 +7,32 @@ ensure our .ini parser is doing what we
python's standard ConfigParser when 2.7 is reality so OrderedDict
is the default:
http://docs.python.org/2/library/configparser.html
"""
import unittest
from manifestparser import read_ini
-from ConfigParser import ConfigParser
from StringIO import StringIO
import mozunit
class IniParserTest(unittest.TestCase):
def test_inline_comments(self):
- """
- We have no inline comments; so we're testing to ensure we don't:
- https://bugzilla.mozilla.org/show_bug.cgi?id=855288
- """
-
- # test '#' inline comments (really, the lack thereof)
- string = """[test_felinicity.py]
+ manifest = """
+[test_felinicity.py]
kittens = true # This test requires kittens
+cats = false#but not cats
"""
- buffer = StringIO()
- buffer.write(string)
- buffer.seek(0)
- result = read_ini(buffer)[0][1]['kittens']
- self.assertEqual(result, "true # This test requires kittens")
-
- # compare this to ConfigParser
- # python 2.7 ConfigParser does not support '#' as an
- # inline comment delimeter (for "backwards compatability"):
- # http://docs.python.org/2/library/configparser.html
- buffer.seek(0)
- parser = ConfigParser()
- parser.readfp(buffer)
- control = parser.get('test_felinicity.py', 'kittens')
- self.assertEqual(result, control)
+ # make sure inline comments get stripped out, but comments without a space in front don't
+ buf = StringIO()
+ buf.write(manifest)
+ buf.seek(0)
+ result = read_ini(buf)[0][1]
+ self.assertEqual(result['kittens'], 'true')
+ self.assertEqual(result['cats'], "false#but not cats")
if __name__ == '__main__':
mozunit.main()