From 7c7238ed37ec0258f1c16b64c292831f0f521c82 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 16:47:03 +0200 Subject: [PATCH 1/9] pretty.py: removed unnecessary call to len() Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index 588afeb6..88e34dbd 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -266,7 +266,7 @@ def _encode_dict(dct, indent, cursor): return "{}" if not _should_wrap(dct, indent): buf = json.dumps(dct) - buf = "{ " + buf[1 : len(buf) - 1] + " }" + buf = "{ " + buf[1:-1] + " }" assert "\n" not in buf if indent + cursor + len(buf) <= MAX_LEN: return buf From ba2411aaa2243e10f124265e490b836b68954a56 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 16:53:53 +0200 Subject: [PATCH 2/9] pretty.py: un-nested functions to decrease complexity Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 178 +++++++++++++++++++++++++------------------------ 1 file changed, 91 insertions(+), 87 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index 88e34dbd..e2ddd555 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -3,6 +3,9 @@ from copy import copy from collections import OrderedDict +MAX_LEN = 80 +INDENT_SIZE = 2 + # Globals for the keys in cfbs.json and their order # Used for validation and prettifying / sorting. TOP_LEVEL_KEYS = ("name", "description", "type", "index", "git", "provides", "build") @@ -216,95 +219,96 @@ def pretty_string(s, sorting_rules=None): return pretty(s, sorting_rules) -def pretty(o, sorting_rules=None): - MAX_LEN = 80 - INDENT_SIZE = 2 +def _should_wrap(parent, indent): + assert isinstance(parent, (tuple, list, dict)) + # We should wrap the top level collection + if indent == 0: + return True + if isinstance(parent, dict): + parent = parent.values() + + count = 0 + for child in parent: + if isinstance(child, (tuple, list, dict)): + if len(child) >= 2: + count += 1 + return count >= 2 + + +def _encode_list(lst, indent, cursor): + if not lst: + return "[]" + if not _should_wrap(lst, indent): + buf = json.dumps(lst) + assert "\n" not in buf + if indent + cursor + len(buf) <= MAX_LEN: + return buf + + indent += INDENT_SIZE + buf = "[\n" + " " * indent + first = True + for value in lst: + if first: + first = False + else: + buf += ",\n" + " " * indent + buf += _encode(value, indent, 0) + indent -= INDENT_SIZE + buf += "\n" + " " * indent + "]" + + return buf + + +def _encode_dict(dct, indent, cursor): + if not dct: + return "{}" + if not _should_wrap(dct, indent): + buf = json.dumps(dct) + buf = "{ " + buf[1:-1] + " }" + assert "\n" not in buf + if indent + cursor + len(buf) <= MAX_LEN: + return buf + + indent += INDENT_SIZE + buf = "{\n" + " " * indent + first = True + for key, value in dct.items(): + if first: + first = False + else: + buf += ",\n" + " " * indent + if not isinstance(key, str): + raise ValueError("Illegal key type '" + type(key).__name__ + "'") + entry = '"' + key + '": ' + buf += entry + _encode(value, indent, len(entry)) + indent -= INDENT_SIZE + buf += "\n" + " " * indent + "}" + + return buf + + +def _encode(data, indent, cursor): + if data is None: + return "null" + elif data is True: + return "true" + elif data is False: + return "false" + elif isinstance(data, (int, float)): + return repr(data) + elif isinstance(data, str): + # Use the json module to escape the string with backslashes: + return json.dumps(data) + elif isinstance(data, (list, tuple)): + return _encode_list(data, indent, cursor) + elif isinstance(data, dict): + return _encode_dict(data, indent, cursor) + else: + raise ValueError("Illegal value type '" + type(data).__name__ + "'") + +def pretty(o, sorting_rules=None): if sorting_rules is not None: _children_sort(o, None, sorting_rules) - def _should_wrap(parent, indent): - assert isinstance(parent, (tuple, list, dict)) - # We should wrap the top level collection - if indent == 0: - return True - if isinstance(parent, dict): - parent = parent.values() - - count = 0 - for child in parent: - if isinstance(child, (tuple, list, dict)): - if len(child) >= 2: - count += 1 - return count >= 2 - - def _encode_list(lst, indent, cursor): - if not lst: - return "[]" - if not _should_wrap(lst, indent): - buf = json.dumps(lst) - assert "\n" not in buf - if indent + cursor + len(buf) <= MAX_LEN: - return buf - - indent += INDENT_SIZE - buf = "[\n" + " " * indent - first = True - for value in lst: - if first: - first = False - else: - buf += ",\n" + " " * indent - buf += _encode(value, indent, 0) - indent -= INDENT_SIZE - buf += "\n" + " " * indent + "]" - - return buf - - def _encode_dict(dct, indent, cursor): - if not dct: - return "{}" - if not _should_wrap(dct, indent): - buf = json.dumps(dct) - buf = "{ " + buf[1:-1] + " }" - assert "\n" not in buf - if indent + cursor + len(buf) <= MAX_LEN: - return buf - - indent += INDENT_SIZE - buf = "{\n" + " " * indent - first = True - for key, value in dct.items(): - if first: - first = False - else: - buf += ",\n" + " " * indent - if not isinstance(key, str): - raise ValueError("Illegal key type '" + type(key).__name__ + "'") - entry = '"' + key + '": ' - buf += entry + _encode(value, indent, len(entry)) - indent -= INDENT_SIZE - buf += "\n" + " " * indent + "}" - - return buf - - def _encode(data, indent, cursor): - if data is None: - return "null" - elif data is True: - return "true" - elif data is False: - return "false" - elif isinstance(data, (int, float)): - return repr(data) - elif isinstance(data, str): - # Use the json module to escape the string with backslashes: - return json.dumps(data) - elif isinstance(data, (list, tuple)): - return _encode_list(data, indent, cursor) - elif isinstance(data, dict): - return _encode_dict(data, indent, cursor) - else: - raise ValueError("Illegal value type '" + type(data).__name__ + "'") - return _encode(o, 0, 0) From 038d0a10c4ca92dce757aacf1ae38968ff5743f6 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 17:00:08 +0200 Subject: [PATCH 3/9] pretty.py: split encoding of list into two functions Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index e2ddd555..c529fdfa 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -235,15 +235,7 @@ def _should_wrap(parent, indent): return count >= 2 -def _encode_list(lst, indent, cursor): - if not lst: - return "[]" - if not _should_wrap(lst, indent): - buf = json.dumps(lst) - assert "\n" not in buf - if indent + cursor + len(buf) <= MAX_LEN: - return buf - +def _encode_list_multiline(lst, indent): indent += INDENT_SIZE buf = "[\n" + " " * indent first = True @@ -255,10 +247,20 @@ def _encode_list(lst, indent, cursor): buf += _encode(value, indent, 0) indent -= INDENT_SIZE buf += "\n" + " " * indent + "]" - return buf +def _encode_list(lst, indent, cursor): + if not lst: + return "[]" + if not _should_wrap(lst, indent): + buf = json.dumps(lst) + assert "\n" not in buf + if indent + cursor + len(buf) <= MAX_LEN: + return buf + return _encode_list_multiline(lst, indent) + + def _encode_dict(dct, indent, cursor): if not dct: return "{}" From 6a5eda98005f0a05c3371ea7e002acdb9abafd58 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 17:01:55 +0200 Subject: [PATCH 4/9] pretty.py: split encoding of dict into two functions Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index c529fdfa..9a0b7ad9 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -261,16 +261,7 @@ def _encode_list(lst, indent, cursor): return _encode_list_multiline(lst, indent) -def _encode_dict(dct, indent, cursor): - if not dct: - return "{}" - if not _should_wrap(dct, indent): - buf = json.dumps(dct) - buf = "{ " + buf[1:-1] + " }" - assert "\n" not in buf - if indent + cursor + len(buf) <= MAX_LEN: - return buf - +def _encode_dict_multiline(dct, indent): indent += INDENT_SIZE buf = "{\n" + " " * indent first = True @@ -285,10 +276,21 @@ def _encode_dict(dct, indent, cursor): buf += entry + _encode(value, indent, len(entry)) indent -= INDENT_SIZE buf += "\n" + " " * indent + "}" - return buf +def _encode_dict(dct, indent, cursor): + if not dct: + return "{}" + if not _should_wrap(dct, indent): + buf = json.dumps(dct) + buf = "{ " + buf[1:-1] + " }" + assert "\n" not in buf + if indent + cursor + len(buf) <= MAX_LEN: + return buf + return _encode_dict_multiline(dct, indent) + + def _encode(data, indent, cursor): if data is None: return "null" From d4b4c7691a61c04763b256afb47d03b1a38547ec Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 17:11:42 +0200 Subject: [PATCH 5/9] pretty.py: implemented single line encoder for lists Ticket: CFE-4571 Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index 9a0b7ad9..82c9f1d1 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -235,6 +235,19 @@ def _should_wrap(parent, indent): return count >= 2 +def _encode_list_single_line(lst, indent, cursor): + buf = "[" + first = True + for child in lst: + if first: + first = False + else: + buf += ", " + buf += _encode(child, indent, cursor + len(buf)) + buf += "]" + return buf + + def _encode_list_multiline(lst, indent): indent += INDENT_SIZE buf = "[\n" + " " * indent @@ -254,8 +267,7 @@ def _encode_list(lst, indent, cursor): if not lst: return "[]" if not _should_wrap(lst, indent): - buf = json.dumps(lst) - assert "\n" not in buf + buf = _encode_list_single_line(lst, indent, cursor) if indent + cursor + len(buf) <= MAX_LEN: return buf return _encode_list_multiline(lst, indent) From 4e18abd6efb64970ba4f5e5e92836a15a8d90d7b Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 17:33:30 +0200 Subject: [PATCH 6/9] pretty.py: implemented single line encoder for dict Ticket: CFE-4571 Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index 82c9f1d1..f23cab7d 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -273,6 +273,22 @@ def _encode_list(lst, indent, cursor): return _encode_list_multiline(lst, indent) +def _encode_dict_single_line(dct, indent, cursor): + buf = "{ " + first = True + for key, value in dct.items(): + if first: + first = False + else: + buf += ", " + if not isinstance(key, str): + raise ValueError("Illegal key type '" + type(key).__name__ + "'") + buf += '"' + key + '": ' + buf += _encode(value, indent, cursor + len(buf)) + buf += " }" + return buf + + def _encode_dict_multiline(dct, indent): indent += INDENT_SIZE buf = "{\n" + " " * indent @@ -295,9 +311,7 @@ def _encode_dict(dct, indent, cursor): if not dct: return "{}" if not _should_wrap(dct, indent): - buf = json.dumps(dct) - buf = "{ " + buf[1:-1] + " }" - assert "\n" not in buf + buf = _encode_dict_single_line(dct, indent, cursor) if indent + cursor + len(buf) <= MAX_LEN: return buf return _encode_dict_multiline(dct, indent) From 7a395d7021b68110cab6fd864d28cc562e1cd90a Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 17:36:05 +0200 Subject: [PATCH 7/9] pretty.py: renamed a variable Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index f23cab7d..3d2041ee 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -252,12 +252,12 @@ def _encode_list_multiline(lst, indent): indent += INDENT_SIZE buf = "[\n" + " " * indent first = True - for value in lst: + for child in lst: if first: first = False else: buf += ",\n" + " " * indent - buf += _encode(value, indent, 0) + buf += _encode(child, indent, 0) indent -= INDENT_SIZE buf += "\n" + " " * indent + "]" return buf From d6da25b95b6a7804b9c49865b60f5f6eaab7ef2c Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 18:16:18 +0200 Subject: [PATCH 8/9] pretty.py: line length wrapping takes comma into consideration Ticket: CFE-4571 Signed-off-by: Lars Erik Wik --- cfbs/pretty.py | 62 ++++++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 32 deletions(-) diff --git a/cfbs/pretty.py b/cfbs/pretty.py index 3d2041ee..def98f9e 100644 --- a/cfbs/pretty.py +++ b/cfbs/pretty.py @@ -237,13 +237,12 @@ def _should_wrap(parent, indent): def _encode_list_single_line(lst, indent, cursor): buf = "[" - first = True - for child in lst: - if first: - first = False - else: + last_index = len(lst) - 1 + for index, child in enumerate(lst): + if index > 0: buf += ", " - buf += _encode(child, indent, cursor + len(buf)) + will_append_comma = index != last_index + buf += _encode(child, indent, cursor + len(buf), will_append_comma) buf += "]" return buf @@ -251,40 +250,39 @@ def _encode_list_single_line(lst, indent, cursor): def _encode_list_multiline(lst, indent): indent += INDENT_SIZE buf = "[\n" + " " * indent - first = True - for child in lst: - if first: - first = False - else: + last_index = len(lst) - 1 + for index, child in enumerate(lst): + if index > 0: buf += ",\n" + " " * indent - buf += _encode(child, indent, 0) + will_append_comma = index != last_index + buf += _encode(child, indent, 0, will_append_comma) indent -= INDENT_SIZE buf += "\n" + " " * indent + "]" return buf -def _encode_list(lst, indent, cursor): +def _encode_list(lst, indent, cursor, will_append_comma): if not lst: return "[]" if not _should_wrap(lst, indent): buf = _encode_list_single_line(lst, indent, cursor) - if indent + cursor + len(buf) <= MAX_LEN: + adjust_for_comma = 1 if will_append_comma else 0 + if (indent + cursor + len(buf)) <= (MAX_LEN - adjust_for_comma): return buf return _encode_list_multiline(lst, indent) def _encode_dict_single_line(dct, indent, cursor): buf = "{ " - first = True - for key, value in dct.items(): - if first: - first = False - else: + last_index = len(dct) - 1 + for index, (key, value) in enumerate(dct.items()): + if index > 0: buf += ", " if not isinstance(key, str): raise ValueError("Illegal key type '" + type(key).__name__ + "'") buf += '"' + key + '": ' - buf += _encode(value, indent, cursor + len(buf)) + will_append_comma = index != last_index + buf += _encode(value, indent, cursor + len(buf), will_append_comma) buf += " }" return buf @@ -292,32 +290,32 @@ def _encode_dict_single_line(dct, indent, cursor): def _encode_dict_multiline(dct, indent): indent += INDENT_SIZE buf = "{\n" + " " * indent - first = True - for key, value in dct.items(): - if first: - first = False - else: + last_index = len(dct) - 1 + for index, (key, value) in enumerate(dct.items()): + if index > 0: buf += ",\n" + " " * indent if not isinstance(key, str): raise ValueError("Illegal key type '" + type(key).__name__ + "'") entry = '"' + key + '": ' - buf += entry + _encode(value, indent, len(entry)) + will_append_comma = index != last_index + buf += entry + _encode(value, indent, len(entry), will_append_comma) indent -= INDENT_SIZE buf += "\n" + " " * indent + "}" return buf -def _encode_dict(dct, indent, cursor): +def _encode_dict(dct, indent, cursor, will_append_comma): if not dct: return "{}" if not _should_wrap(dct, indent): buf = _encode_dict_single_line(dct, indent, cursor) - if indent + cursor + len(buf) <= MAX_LEN: + adjust_for_comma = 1 if will_append_comma else 0 + if (indent + cursor + len(buf)) <= (MAX_LEN - adjust_for_comma): return buf return _encode_dict_multiline(dct, indent) -def _encode(data, indent, cursor): +def _encode(data, indent, cursor, will_append_comma): if data is None: return "null" elif data is True: @@ -330,9 +328,9 @@ def _encode(data, indent, cursor): # Use the json module to escape the string with backslashes: return json.dumps(data) elif isinstance(data, (list, tuple)): - return _encode_list(data, indent, cursor) + return _encode_list(data, indent, cursor, will_append_comma) elif isinstance(data, dict): - return _encode_dict(data, indent, cursor) + return _encode_dict(data, indent, cursor, will_append_comma) else: raise ValueError("Illegal value type '" + type(data).__name__ + "'") @@ -341,4 +339,4 @@ def pretty(o, sorting_rules=None): if sorting_rules is not None: _children_sort(o, None, sorting_rules) - return _encode(o, 0, 0) + return _encode(o, 0, 0, False) From d2a3e40ae381351bedf4c6e14080f13fbebddac7 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Wed, 27 Aug 2025 18:18:14 +0200 Subject: [PATCH 9/9] Added unit tests comparing cfbs pretty with output from npm prettier We saw some cases where cfbs pretty and npm prettier did not agree. These tests prove that they now agree for those specific cases. Ticket: CFE-4571 Signed-off-by: Lars Erik Wik --- tests/test_pretty.py | 83 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/tests/test_pretty.py b/tests/test_pretty.py index f3c5bd04..8ee23e42 100644 --- a/tests/test_pretty.py +++ b/tests/test_pretty.py @@ -516,3 +516,86 @@ def test_pretty_sorting_real_examples(): }""" assert pretty_string(test_json, cfbs_sorting_rules) == expected + + +def test_pretty_same_as_npm_prettier(): + # We saw some cases where cfbs pretty and npm prettier did not agree + # Testing that this is no longer the case + + test_json = """ + { + "classes": { "My_class": {}, "My_class2": {"comment": "comment body"} } + } + """ + + expected = """{ + "classes": { "My_class": {}, "My_class2": { "comment": "comment body" } } +}""" + + assert pretty_string(test_json) == expected + + test_json = """ + { + "filter": { + "filter": { "Attribute name": {"operator": "value2"} }, + "hostFilter": { + "includes": { + "includeAdditionally": false, + "entries": { + "ip": ["192.168.56.5"], + "hostkey": [], + "hostname": ["ubuntu-bionic"], + "mac": ["08:00:27:0b:a4:99", "08:00:27:dd:e1:59", "02:9f:d3:59:7e:90"], + "ip_mask": ["10.0.2.16/16"] + } + }, + "excludes": { + "entries": { + "ip": [], + "hostkey": [], + "hostname": [], + "mac": [], + "ip_mask": [] + } + } + }, + "hostContextExclude": ["class_value"], + "hostContextInclude": ["class_value"] + } + } + """ + + expected = """{ + "filter": { + "filter": { "Attribute name": { "operator": "value2" } }, + "hostFilter": { + "includes": { + "includeAdditionally": false, + "entries": { + "ip": ["192.168.56.5"], + "hostkey": [], + "hostname": ["ubuntu-bionic"], + "mac": [ + "08:00:27:0b:a4:99", + "08:00:27:dd:e1:59", + "02:9f:d3:59:7e:90" + ], + "ip_mask": ["10.0.2.16/16"] + } + }, + "excludes": { + "entries": { + "ip": [], + "hostkey": [], + "hostname": [], + "mac": [], + "ip_mask": [] + } + } + }, + "hostContextExclude": ["class_value"], + "hostContextInclude": ["class_value"] + } +}""" + + assert pretty_string(test_json) == expected