Repository: ebroecker/canmatrix

Issue:
"Title" field in .sym files not processed
One of my use cases is to read .sym files modify them in python and then to export them again. The title field is written by PCAN symbol editor and I think it makes sense to have it be preserved if the file if read and then written back to file.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/ebroecker/canmatrix/pull/470?src=pr&el=h1) Report
> Merging [#470](https://codecov.io/gh/ebroecker/canmatrix/pull/470?src=pr&el=desc) into [development](https://codecov.io/gh/ebroecker/canmatrix/commit/09c32eaff27d4957e2b1c819231d11cb891a999a&el=desc) will **increase** coverage by `0.07%`.
> The diff coverage is `93.33%`.

[![Impacted file tree graph](https://codecov.io/gh/ebroecker/canmatrix/pull/470/graphs/tree.svg?width=650&height=150&src=pr&token=I5J8CV2skH)](https://codecov.io/gh/ebroecker/canmatrix/pull/470?src=pr&el=tree)

```diff
@@               Coverage Diff               @@
##           development     #470      +/-   ##
===============================================
+ Coverage        59.46%   59.54%   +0.07%     
===============================================
  Files               45       45              
  Lines             8132     8145      +13     
  Branches          1792     1793       +1     
===============================================
+ Hits              4836     4850      +14     
+ Misses            2919     2916       -3     
- Partials           377      379       +2     
```


| [Impacted Files](https://codecov.io/gh/ebroecker/canmatrix/pull/470?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/canmatrix/canmatrix.py](https://codecov.io/gh/ebroecker/canmatrix/pull/470/diff?src=pr&el=tree#diff-c3JjL2Nhbm1hdHJpeC9jYW5tYXRyaXgucHk=) | `84.87% <50.00%> (+0.21%)` | :arrow_up: |
| [src/canmatrix/formats/sym.py](https://codecov.io/gh/ebroecker/canmatrix/pull/470/diff?src=pr&el=tree#diff-c3JjL2Nhbm1hdHJpeC9mb3JtYXRzL3N5bS5weQ==) | `49.36% <100.00%> (+0.64%)` | :arrow_up: |
| [src/canmatrix/tests/test\_sym.py](https://codecov.io/gh/ebroecker/canmatrix/pull/470/diff?src=pr&el=tree#diff-c3JjL2Nhbm1hdHJpeC90ZXN0cy90ZXN0X3N5bS5weQ==) | `96.42% <100.00%> (+0.51%)` | :arrow_up: |
| [src/canmatrix/formats/dbc.py](https://codecov.io/gh/ebroecker/canmatrix/pull/470/diff?src=pr&el=tree#diff-c3JjL2Nhbm1hdHJpeC9mb3JtYXRzL2RiYy5weQ==) | `72.31% <0.00%> (-0.15%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/ebroecker/canmatrix/pull/470?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/ebroecker/canmatrix/pull/470?src=pr&el=footer). Last update [09c32ea...c8a835b](https://codecov.io/gh/ebroecker/canmatrix/pull/470?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/canmatrix/canmatrix.py b/src/canmatrix/canmatrix.py
index cbeef4a..355b7c3 100644
--- a/src/canmatrix/canmatrix.py
+++ b/src/canmatrix/canmatrix.py
@@ -1485,10 +1485,11 @@ class CanMatrix(object):
         """
         if attributeName in self.attributes:
             return self.attributes[attributeName]
-        else:
-            if attributeName in self.global_defines:
+        elif attributeName in self.global_defines:
                 define = self.global_defines[attributeName]
                 return define.defaultValue
+        else:
+            return default
 
     def add_value_table(self, name, valueTable):  # type: (str, typing.Mapping) -> None
         """Add named value table.
diff --git a/src/canmatrix/formats/sym.py b/src/canmatrix/formats/sym.py
index 3076cdf..a67d137 100644
--- a/src/canmatrix/formats/sym.py
+++ b/src/canmatrix/formats/sym.py
@@ -181,9 +181,10 @@ def dump(db, f, **options):  # type: (canmatrix.CanMatrix, typing.IO, **typing.A
     enum_dict = {}
     enums = "{ENUMS}\n"
 
-    header = """FormatVersion=5.0 // Do not edit this line!
-Title=\"canmatrix-Export\"
-"""
+    header = """\
+FormatVersion=5.0 // Do not edit this line!
+Title=\"{}\"
+""".format(db.attribute("Title", "canmatrix-Export"))
     f.write(header.encode(sym_encoding, ignore_encoding_errors))
 
     def send_receive(for_frame):
@@ -330,7 +331,11 @@ def load(f, **options):  # type: (typing.IO, **typing.Any) -> canmatrix.CanMatri
             # ignore empty line:
             if line.__len__() == 0:
                 continue
-
+            if line[0:6] == "Title=":
+                title = line[6:].strip('"')
+                db.add_global_defines("Title", "STRING")
+                db.global_defines['Title'].set_default("canmatrix-Export")
+                db.add_attribute("Title", title)
             # switch mode:
             if line[0:7] == "{ENUMS}":
                 mode = Mode.enums
@@ -363,7 +368,7 @@ def load(f, **options):  # type: (typing.IO, **typing.Any) -> canmatrix.CanMatri
                     line = line.split('//')[0]
                     temp_array = line[5:].strip().rstrip(')').split('(', 1)
                     val_table_name = temp_array[0]
-                    split = canmatrix.utils.quote_aware_space_split(temp_array[1])
+                    split = canmatrix.utils.quote_aware_comma_split(temp_array[1])
                     temp_array = [s.rstrip(',') for s in split]
                     temp_val_table = {}
                     for entry in temp_array:
diff --git a/src/canmatrix/utils.py b/src/canmatrix/utils.py
index 5d9e309..57ad792 100644
--- a/src/canmatrix/utils.py
+++ b/src/canmatrix/utils.py
@@ -21,11 +21,37 @@ def quote_aware_space_split(in_line):  # type: (str) -> typing.List[str]
 
 
 def quote_aware_comma_split(string):  # type: (str) -> typing.List[str]
-    if sys.version_info >= (3, 0):
-        temp = list(csv.reader([string], skipinitialspace=True))
-    else:
-        temp = list(csv.reader([string.encode("utf8")], skipinitialspace=True))
-    return temp[0]
+    """
+    Split a string containing comma separated list of fields.
+    Removing surrounding whitespace, to allow fields to be separated by ", ".
+    Preserves double quotes within fields, but not double quotes surrounding fields.
+    Suppresses comma separators which are within double quoted sections.
+    :param string: ('a,  b", c", "d"',
+    :return: ['a', 'b", c"', 'd']),
+    """
+    fields = []
+    quoted = False
+    field = ""
+    # Separate string by unquoted commas
+    for char in string:
+        if char == ',':
+            if not quoted:
+                fields.append(field)
+                field = ""
+                continue
+        if char == '"':
+            quoted = not quoted
+        field += char
+    if field:
+        fields.append(field)
+    # Remove surrounding whitespace from fields
+    fields = [f.strip() for f in fields]
+    # Remove "" that surround entire fields
+    for i, f in enumerate(fields):
+        if len(f) > 1:
+            if f.startswith('"') and f.endswith('"'):
+                fields[i] = f[1:-1]
+    return fields
 
 
 def guess_value(text_value):  # type: (str) -> str


Test patch:
diff --git a/src/canmatrix/tests/test_sym.py b/src/canmatrix/tests/test_sym.py
index 962747e..d5b5ccd 100644
--- a/src/canmatrix/tests/test_sym.py
+++ b/src/canmatrix/tests/test_sym.py
@@ -173,3 +173,48 @@ def test_unterminated_enum():
     else:
         assert isinstance(matrix.load_errors[0], StopIteration)
 
+
+def test_title_read_and_write():
+    f = io.BytesIO(
+        textwrap.dedent(
+            '''\
+            FormatVersion=5.0 // Do not edit this line!
+            Title="An Example Title"
+
+            '''
+        ).encode('utf-8'),
+    )
+
+    matrix = canmatrix.formats.sym.load(f)
+    assert matrix.attribute("Title") == "An Example Title"
+    f_out = io.BytesIO()
+    canmatrix.formats.sym.dump(matrix, f_out)
+    assert f_out.getvalue().decode('utf-8').splitlines()[1] == 'Title="An Example Title"'
+
+@pytest.mark.parametrize(
+    'enum_str, enum_dict, enum_label',
+    (
+        ('enum Animal(0="Dog", 1="Cat", 2="Fox")', {"Animal": {0: "Dog", 1: "Cat", 2: "Fox"}}, "Simple enum"),
+        ('''\
+enum Animal(0="Dog", //A Comment
+1="Cat",
+2="Fox")''',
+         {"Animal": {0: "Dog", 1: "Cat", 2: "Fox"}}, "Multiline enum"),
+        ('enum Animal(0="Dog",1="Cat",2="Fox")', {"Animal": {0: "Dog", 1: "Cat", 2: "Fox"}}, "No Space  in Separator"),
+    )
+)
+def test_enums_read(enum_str, enum_dict, enum_label):
+        f = io.BytesIO('''\
+FormatVersion=5.0 // Do not edit this line!
+Title="An Example Title"
+
+{{ENUMS}}
+{}
+'''.format(enum_str).encode('utf-8'),
+        )
+
+        matrix = canmatrix.formats.sym.load(f)
+        assert matrix.load_errors == [], "Failed to load canmatrix, when testing enum case : '{}'".format(enum_label)
+        assert matrix.value_tables == enum_dict, "Enum not parsed correctly : '{}'".format(enum_label)
+        f_out = io.BytesIO()
+        canmatrix.formats.sym.dump(matrix, f_out)
diff --git a/src/canmatrix/tests/test_utils.py b/src/canmatrix/tests/test_utils.py
index 356990c..2aad410 100644
--- a/src/canmatrix/tests/test_utils.py
+++ b/src/canmatrix/tests/test_utils.py
@@ -1,4 +1,6 @@
 # -*- coding: utf-8 -*-
+import pytest
+
 import canmatrix.utils
 
 
@@ -10,7 +12,31 @@ def test_utils_guess_value():
     assert canmatrix.utils.guess_value("False") == "0"
     assert canmatrix.utils.guess_value("faLse") == "0"
 
+
 def test_decode_number():
     assert canmatrix.utils.decode_number("0x10") == 16
     assert canmatrix.utils.decode_number("0b10") == 2
     assert canmatrix.utils.decode_number("10") == 10
+
+
+@pytest.mark.parametrize(
+    'input_string, expected_list',
+    (
+            ('a,b,c,d',
+             ["a", "b", "c", "d"]),
+
+            ('a,  b, c, d',
+             ["a", "b", "c", "d"]),
+
+            ('a,  b", c", "d"',
+             ['a', 'b", c"', 'd']),
+
+            ('0="a",  1=b, 3="c"d, 4=e',
+             ['0="a"',  '1=b', '3="c"d', '4=e']),
+
+            ('"a,b",","b,c","\'\'d"e',
+             ['a,b', '","b', 'c","\'\'d\"e']),
+    )
+)
+def test_quote_aware_comma_split_function(input_string, expected_list):
+    assert canmatrix.utils.quote_aware_comma_split(input_string) == expected_list