[master/f21 v3] Fix # handling in SimpleConfigFile (#1045687)
Anne Mulhern
amulhern at redhat.com
Wed Oct 15 12:39:05 UTC 2014
----- Original Message -----
> From: "Brian C. Lane" <bcl at redhat.com>
> To: anaconda-patches at lists.fedorahosted.org
> Sent: Tuesday, October 14, 2014 5:48:07 PM
> Subject: [master/f21 v3] Fix # handling in SimpleConfigFile (#1045687)
>
> It was treating anything after a # as a comment, even if it appeared
> inside a quoted string. This fixes it to find the last # outside a
> (optional) quoted string and adds several tests.
>
> This also adds back the non-IfcfgFile tests for SimpleConfigFile from
> old_tests
> ---
> pyanaconda/simpleconfig.py | 63 ++++++++++----
> tests/pyanaconda_tests/simpleconfig_test.py | 130
> ++++++++++++++++++++++++++++
> 2 files changed, 177 insertions(+), 16 deletions(-)
> create mode 100644 tests/pyanaconda_tests/simpleconfig_test.py
>
> diff --git a/pyanaconda/simpleconfig.py b/pyanaconda/simpleconfig.py
> index a148384..1a9751c 100644
> --- a/pyanaconda/simpleconfig.py
> +++ b/pyanaconda/simpleconfig.py
> @@ -43,6 +43,30 @@ def quote(s, always=False):
> return s
> return '"'+s.replace('"', '\\"')+'"'
>
> +def find_comment(s):
> + """ Look for a # comment outside of a quoted string.
> + If there are no quotes, find the last # in the string.
> +
> + :param str s: string to check for comment and quotes
> + :returns: index of comment or None
> + :rtype: int or None
> +
> + Handles comments inside quotes and quotes inside quotes.
> + """
> + q = None
> + for i in range(len(s)):
> + if not q and s[i] == '#':
> + return i
> +
> + # Ignore quotes inside other quotes
> + if s[i] in "'\"":
> + if s[i] == q:
> + q = None
> + elif q is None:
> + q = s[i]
> + return None
> +
> +
> class SimpleConfigFile(object):
> """ Edit values in a configuration file without changing comments.
> Supports KEY=VALUE lines and ignores everything else.
> @@ -73,7 +97,7 @@ class SimpleConfigFile(object):
> with open(filename) as f:
> for line in f:
> self._lines.append(line)
> - key, value = self._parseline(line)
> + key, value, _comment = self._parseline(line)
> if key:
> self.info[key] = value
>
> @@ -116,27 +140,40 @@ class SimpleConfigFile(object):
> return self.info.get(upperASCII(key), "")
>
> def _parseline(self, line):
> - """ parse a line into a key, value pair
> + """ parse a line into a key, value and comment
> +
> + :param str line: Line to be parsed
> + :returns: Tuple of key, value, comment
> + :rtype: tuple
> +
> Handle comments and optionally unquote quoted strings
> - Returns (key, value) or (None, None)
> - key is always UPPERCASE
> + Returns (key, value, comment) or (None, None, comment)
> + key is always UPPERCASE and comment may by "" if none was found.
> """
> s = line.strip()
> - if '#' in s:
> - s = s[:s.find('#')] # remove from comment to EOL
> - s = s.strip() # and any unnecessary whitespace
> + # Look for a # outside any quotes
> + comment = ""
> + comment_index = find_comment(s)
> + if comment_index is not None:
> + comment = s[comment_index:]
> + s = s[:comment_index] # remove from comment to EOL
> + s = s.strip() # and any unnecessary whitespace
You should lose the conditional strip() above.
> +
> key, eq, val = s.partition('=')
> + key = key.strip()
But add:
val = val.strip()
here, unless leading whitespace is significant in val, which seems unlikely.
> if self.read_unquote:
> val = unquote(val)
> if key != '' and eq == '=':
> - return (upperASCII(key), val)
> + return (upperASCII(key), val, comment)
> else:
> - return (None, None)
> + return (None, None, comment)
>
> def _kvpair(self, key, comment=""):
> value = self.info[key]
> if self.write_quote or self.always_quote:
> value = quote(value, self.always_quote)
> + if comment:
> + comment = " " + comment
> return key + '=' + value + comment + "\n"
Above three lines could be:
padding = " " if comment else ""
return key + "=" + value + padding + comment + "\n"
>
> def __str__(self):
> @@ -146,17 +183,13 @@ class SimpleConfigFile(object):
> oldkeys = []
> s = ""
> for line in self._lines:
> - key = self._parseline(line)[0]
> + key, _value, comment = self._parseline(line)
> if key is None:
> s += line
> else:
> if key not in self.info:
> continue
> oldkeys.append(key)
> - if "#" in line:
> - comment = " " + line[line.find("#"):]
> - else:
> - comment = ""
> s += self._kvpair(key, comment)
>
> # Add new keys
> @@ -165,5 +198,3 @@ class SimpleConfigFile(object):
> s += self._kvpair(key)
>
> return s
> -
> -
> diff --git a/tests/pyanaconda_tests/simpleconfig_test.py
> b/tests/pyanaconda_tests/simpleconfig_test.py
> new file mode 100644
> index 0000000..3719ec1
> --- /dev/null
> +++ b/tests/pyanaconda_tests/simpleconfig_test.py
> @@ -0,0 +1,130 @@
> +# -*- coding: utf-8 -*-
> +#
> +# Copyright (C) 2014 Red Hat, Inc.
> +#
> +# This copyrighted material is made available to anyone wishing to use,
> +# modify, copy, or redistribute it subject to the terms and conditions of
> +# the GNU General Public License v.2, or (at your option) any later version.
> +# This program is distributed in the hope that it will be useful, but
> WITHOUT
> +# ANY WARRANTY expressed or implied, including the implied warranties of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General
> +# Public License for more details. You should have received a copy of the
> +# GNU General Public License along with this program; if not, write to the
> +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> MA
> +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the
> +# source code or documentation are not subject to the GNU General Public
> +# License and may only be used or replicated with the express permission of
> +# Red Hat, Inc.
> +#
> +# Red Hat Author(s): Brian C. Lane <bcl at redhat.com>
> +
> +from pyanaconda.simpleconfig import SimpleConfigFile
> +from pyanaconda import simpleconfig
> +import unittest
> +import tempfile
> +
> +class SimpleConfigTests(unittest.TestCase):
> + TEST_CONFIG = """ESSID="Example Network #1"
> +ESSID2="Network #2" # With a comment
> +COMMENT="Save this string" # Strip this comment
> +#SKIP=Skip this commented line
> +BOOT=always
> +KEY=VALUE # Comment "with quotes"
> +KEY2="A single ' inside" # And comment "with quotes"
> +"""
> +
> + def comment_test(self):
> + with tempfile.NamedTemporaryFile() as testconfig:
> + testconfig.write(self.TEST_CONFIG)
> + testconfig.flush()
> +
> + config = SimpleConfigFile(testconfig.name)
> + config.read()
> + self.assertEqual(config.get("ESSID"), "Example Network #1")
> + self.assertEqual(config.get("ESSID2"), "Network #2")
> + self.assertEqual(config.get("COMMENT"), "Save this string")
> + self.assertEqual(str(config), self.TEST_CONFIG)
> +
> + def unquote_test(self):
> + self.assertEqual(simpleconfig.unquote("plain string"), "plain
> string")
> + self.assertEqual(simpleconfig.unquote('"double quote"'), "double
> quote")
> + self.assertEqual(simpleconfig.unquote("'single quote'"), "single
> quote")
> +
> + def quote_test(self):
> + self.assertEqual(simpleconfig.quote("nospaces"), "nospaces")
> + self.assertEqual(simpleconfig.quote("plain string"), '"plain
> string"')
> + self.assertEqual(simpleconfig.quote("alwaysquote", always=True),
> '"alwaysquote"')
> +
> + def set_and_get_test(self):
> + """Setting and getting values"""
> + scf = SimpleConfigFile()
> + scf.set(('key1', 'value1'))
> + self.assertEqual(scf.get('key1'), 'value1')
> + scf.set(('KEY2', 'value2'))
> + self.assertEqual(scf.get('key2'), 'value2')
> + scf.set(('KEY3', 'value3'))
> + self.assertEqual(scf.get('KEY3'), 'value3')
> + scf.set(('key4', 'value4'))
> + self.assertEqual(scf.get('KEY4'), 'value4')
> +
> + def unset_test(self):
> + scf = SimpleConfigFile()
> + scf.set(('key1', 'value1'))
> + scf.unset(('key1'))
> + self.assertEqual(scf.get('key1'), '')
> +
> + def write_test(self):
> + with tempfile.NamedTemporaryFile() as testconfig:
> + scf = SimpleConfigFile()
> + scf.set(('key1', 'value1'))
> + scf.write(testconfig.name)
> + testconfig.flush()
> + self.assertEqual(open(testconfig.name).read(), 'KEY1=value1\n')
> +
> + def read_test(self):
> + with tempfile.NamedTemporaryFile() as testconfig:
> + scf = SimpleConfigFile()
> + open(testconfig.name, 'w').write('KEY1="value1"\n')
> + testconfig.flush()
> + scf.read(testconfig.name)
> + self.assertEqual(scf.get('key1'), 'value1')
> +
> + def read_write_test(self):
> + with tempfile.NamedTemporaryFile() as testconfig:
> + testconfig.write(self.TEST_CONFIG)
> + testconfig.flush()
> +
> + scf = SimpleConfigFile()
> + scf.read(testconfig.name)
> + scf.write(testconfig.name)
> + testconfig.flush()
> + self.assertEqual(open(testconfig.name).read(), self.TEST_CONFIG)
> +
> + def write_new_keys_test(self):
> + with tempfile.NamedTemporaryFile() as testconfig:
> + testconfig.write(self.TEST_CONFIG)
> + testconfig.flush()
> +
> + scf = SimpleConfigFile()
> + scf.read(testconfig.name)
> + scf.set(("key1", "value1"))
> + scf.write(testconfig.name)
> + testconfig.flush()
> +
> + self.assertEqual(open(testconfig.name).read(),
> + self.TEST_CONFIG+"KEY1=value1\n")
> +
> + def remove_key_test(self):
> + with tempfile.NamedTemporaryFile() as testconfig:
> + testconfig.write(self.TEST_CONFIG)
> + testconfig.flush()
> +
> + scf = SimpleConfigFile()
> + scf.read(testconfig.name)
> + self.assertEqual(scf.get("BOOT"), "always")
> + scf.unset("BOOT")
> + scf.write(testconfig.name)
> + testconfig.flush()
> + scf.reset()
> + scf.read(testconfig.name)
> + self.assertEqual(scf.get("BOOT"), "")
> --
> 1.9.3
>
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
Other than two inline comments, looks good.
- mulhern
More information about the anaconda-patches
mailing list