[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