From 33e279e2a909d96430eb84ce9004eb7ca9b822e8 Mon Sep 17 00:00:00 2001 From: gaoflow Date: Wed, 17 Jun 2026 11:52:34 +0200 Subject: [PATCH 1/2] fix comments being silently dropped when a collection is rendered inline The dumper promises (doc/comments.rst, releases.rst) that comments are re-emitted when the same keymap is supplied. The multiline path already guards this via _comments_force_multiline, forcing a value onto its own line so its comments are not lost. The inline path (render_value's {...}/[...] branches) had no such guard, so any comment on an inlined collection's value slots or on its descendants was dropped with no warning. Add _inline_would_drop_comments, mirroring _comments_force_multiline, and raise NotSuitableForInline (the existing inline->multiline fallback) when inlining would drop a comment. --- nestedtext/nestedtext.py | 47 ++++++++++++++++++++++++++++++++++++++++ tests/test_comments.py | 30 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/nestedtext/nestedtext.py b/nestedtext/nestedtext.py index 78c369b..cf92a5d 100644 --- a/nestedtext/nestedtext.py +++ b/nestedtext/nestedtext.py @@ -2195,6 +2195,49 @@ def _comments_force_multiline(self, keys): return True return False + # _inline_would_drop_comments {{{3 + def _inline_would_drop_comments(self, keys): + """Return True if rendering the collection at *keys* inline would + silently drop a comment. + + The inline forms (``{...}`` / ``[...]``) emit no comments for the + collection's own value slots nor for any of its descendants, so + inlining is only safe when neither carries any comment. Comments + on the collection's *key* (the ``key_leading``/``key_trailing`` + slots at *keys* itself) are still emitted by the parent, so they + do not force multi-line here; only value-side comments at *keys* + and any comment (or comment provider) at a strict descendant do. + """ + if not is_mapping(self.map_keys): + return False + loc = self.map_keys.get(keys) + if loc is not None: + if loc.get_value_leading_comments() or loc.get_value_trailing_comments(): + return True + if ( + loc.get_key_leading_provider() is not None + or loc.get_key_trailing_provider() is not None + or loc.get_value_leading_provider() is not None + or loc.get_value_trailing_provider() is not None + ): + return True + depth = len(keys) + for path, loc in self.map_keys.items(): + if len(path) <= depth or path[:depth] != keys: + continue + if ( + loc.get_key_leading_comments() + or loc.get_key_trailing_comments() + or loc.get_value_leading_comments() + or loc.get_value_trailing_comments() + or loc.get_key_leading_provider() is not None + or loc.get_key_trailing_provider() is not None + or loc.get_value_leading_provider() is not None + or loc.get_value_trailing_provider() is not None + ): + return True + return False + # render_inline_value {{{3 def render_inline_value(self, obj, exclude, keys, values): obj = self.convert(obj, keys) @@ -2447,6 +2490,8 @@ def render_value(self, obj, keys, values): raise NotSuitableForInline from None if obj and (self.width <= 0 or level < self.inline_level): raise NotSuitableForInline from None + if self._inline_would_drop_comments(keys): + raise NotSuitableForInline from None try: if 0 < len(obj) < self.inline_count: raise NotSuitableForInline from None @@ -2479,6 +2524,8 @@ def render_value(self, obj, keys, values): raise NotSuitableForInline from None if obj and (self.width <= 0 or level < self.inline_level): raise NotSuitableForInline from None + if self._inline_would_drop_comments(keys): + raise NotSuitableForInline from None try: if 0 < len(obj) < self.inline_count: raise NotSuitableForInline from None diff --git a/tests/test_comments.py b/tests/test_comments.py index dc7ade4..455fcc6 100644 --- a/tests/test_comments.py +++ b/tests/test_comments.py @@ -1668,3 +1668,33 @@ def test_comment_repr_includes_new_fields_when_set(): assert "tab=" not in r2 assert "before=" not in r2 assert "after=" not in r2 + + +# --------------------------------------------------------------------------- +# Inlining must not silently drop comments +# --------------------------------------------------------------------------- + +def test_inline_does_not_drop_comment_on_list_item(): + """A collection that carries comments on its items must fall back to the + multi-line form rather than inline, otherwise the comments are silently + dropped (the inline forms have no place to emit them).""" + data = {"servers": ["alpha", "beta"]} + keymap = {} + annotate(("servers", 0), keymap, key_leading=[Comment("the first server")]) + # width is large enough that, absent comments, the list would be inlined. + out = nt.dumps(data, map_keys=keymap, width=200, inline_level=0, inline_count=1) + assert "the first server" in out + # and the list was forced to the multi-line form + assert "- alpha" in out + assert "[alpha, beta]" not in out + + +def test_inline_does_not_drop_value_comment_on_dict(): + """A value-side comment on a key whose dict value would inline must keep + that value multi-line so the comment survives.""" + data = {"config": {"a": "1", "b": "2"}} + keymap = {} + annotate(("config",), keymap, value_trailing=[Comment("end of config", tab=1)]) + out = nt.dumps(data, map_keys=keymap, width=200, inline_level=0, inline_count=1) + assert "end of config" in out + assert "{a: 1, b: 2}" not in out From 040b299b268cdb4fc83bac145bdd4a8654e64943 Mon Sep 17 00:00:00 2001 From: Vincent Gao Date: Sat, 20 Jun 2026 10:16:26 +0200 Subject: [PATCH 2/2] Compute inline comment-drop check once per dump (O(N), not O(N^2)) _inline_would_drop_comments rescanned all of map_keys for every collection considered for inlining, which is O(N^2) over the document. Build the set of unsafe-to-inline keys in a single pass over map_keys and cache it on the dumper, so each check is an O(1) membership test. Behaviour is unchanged: the comment suite (139) and the official-corpus round-trip tests (1837) pass, and a deep-comment dump now scales linearly with document size instead of quadratically. --- nestedtext/nestedtext.py | 54 ++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/nestedtext/nestedtext.py b/nestedtext/nestedtext.py index cf92a5d..18e86f1 100644 --- a/nestedtext/nestedtext.py +++ b/nestedtext/nestedtext.py @@ -2078,6 +2078,7 @@ def __init__( self.indent = indent self.converters = converters self.map_keys = map_keys + self._inline_unsafe_keys = None # cached set, built lazily per dump self.default = default self.spacing = spacing or {} self.width = width @@ -2207,36 +2208,57 @@ def _inline_would_drop_comments(self, keys): slots at *keys* itself) are still emitted by the parent, so they do not force multi-line here; only value-side comments at *keys* and any comment (or comment provider) at a strict descendant do. + + The set of keys for which this holds is built once per dump in a + single pass over *map_keys* (see `_compute_inline_unsafe_keys`) and + cached, so each query is an O(1) membership test rather than a + rescan of every entry for every collection. """ if not is_mapping(self.map_keys): return False - loc = self.map_keys.get(keys) - if loc is not None: - if loc.get_value_leading_comments() or loc.get_value_trailing_comments(): - return True - if ( + if self._inline_unsafe_keys is None: + self._inline_unsafe_keys = self._compute_inline_unsafe_keys() + return keys in self._inline_unsafe_keys + + # _compute_inline_unsafe_keys {{{3 + def _compute_inline_unsafe_keys(self): + """Collect, in a single pass over *map_keys*, every *keys* for which + inlining the collection there would drop a comment. + + A comment (or comment provider) at some path *p* makes inlining + unsafe at: every strict ancestor of *p* (the inline form has nowhere + to emit a descendant's comment); and at *p* itself when the comment + is value-side (key-side comments at *p* are emitted by *p*'s parent). + Providers at *p* count for both, matching `_inline_would_drop_comments`. + """ + unsafe = set() + for path, loc in self.map_keys.items(): + has_provider = ( loc.get_key_leading_provider() is not None or loc.get_key_trailing_provider() is not None or loc.get_value_leading_provider() is not None or loc.get_value_trailing_provider() is not None + ) + # value-side comment or any provider at *path* makes inlining the + # collection at *path* itself unsafe + if ( + loc.get_value_leading_comments() + or loc.get_value_trailing_comments() + or has_provider ): - return True - depth = len(keys) - for path, loc in self.map_keys.items(): - if len(path) <= depth or path[:depth] != keys: - continue + unsafe.add(path) + # any comment or provider at *path* makes inlining any strict + # ancestor of *path* unsafe if ( loc.get_key_leading_comments() or loc.get_key_trailing_comments() or loc.get_value_leading_comments() or loc.get_value_trailing_comments() - or loc.get_key_leading_provider() is not None - or loc.get_key_trailing_provider() is not None - or loc.get_value_leading_provider() is not None - or loc.get_value_trailing_provider() is not None + or has_provider ): - return True - return False + for depth in range(len(path)): + unsafe.add(path[:depth]) + return unsafe # render_inline_value {{{3 def render_inline_value(self, obj, exclude, keys, values):