pipenv's Trivia patch to make tomlkit dump toml's inline table

[jayvdb]

pipenv applies a patch to their vendored copy of tomlkit, and it seems that it doesnt exist here yet.

The patch is https://github.com/pypa/pipenv/blob/master/tasks/vendoring/patches/vendor/tomlkit-fix.patch

Parts of that have been merged, but the bit that looks missing is:

diff -ru tomlkit-0.5.3-orig/tomlkit/container.py tomlkit-0.5.3/tomlkit/container.py
--- tomlkit-0.5.3-orig/tomlkit/container.py	2018-11-14 23:10:40.697032200 +0700
+++ tomlkit-0.5.3/tomlkit/container.py	2019-03-14 10:57:20.658602016 +0700
@@ -19,6 +19,7 @@
 from .items import Key
 from .items import Null
 from .items import Table
+from .items import Trivia
 from .items import Whitespace
 from .items import item as _item
 
@@ -223,7 +224,12 @@
             for i in idx:
                 self._body[i] = (None, Null())
         else:
-            self._body[idx] = (None, Null())
+            old_data = self._body[idx][1]
+            trivia = getattr(old_data, "trivia", None)
+            if trivia and trivia.comment:
+                self._body[idx] = (None, Comment(Trivia(comment_ws="", comment=trivia.comment)))
+            else:
+                self._body[idx] = (None, Null())
 
         super(Container, self).__delitem__(key.key)
 
diff -ru tomlkit-0.5.3-orig/tomlkit/items.py tomlkit-0.5.3/tomlkit/items.py
--- tomlkit-0.5.3-orig/tomlkit/items.py	2018-11-20 01:11:57.965421000 +0700
+++ tomlkit-0.5.3/tomlkit/items.py	2019-03-14 10:59:38.451740952 +0700
@@ -20,6 +20,7 @@
 from ._compat import long
 from ._compat import unicode
 from ._utils import escape_string
+from toml.decoder import InlineTableDict
 
 if PY2:
     from functools32 import lru_cache
@@ -40,7 +41,10 @@
     elif isinstance(value, float):
         return Float(value, Trivia(), str(value))
     elif isinstance(value, dict):
-        val = Table(Container(), Trivia(), False)
+        if isinstance(value, InlineTableDict):
+            val = InlineTable(Container(), Trivia())
+        else:
+            val = Table(Container(), Trivia(), False)
         for k, v in sorted(value.items(), key=lambda i: (isinstance(i[1], dict), i[0])):
             val[k] = item(v, _parent=val)
 

Is this an appropriate addition to tomlkit?

It appears the author was @frostming , originally at https://github.com/pypa/pipenv/commits/6df7d8861da841e552049dcde9ff9a0f23edc01e/tasks/vendoring/patches/vendor/tomlkit-dump-inline-table.patch

I dont see any similar patch in https://github.com/sdispater/tomlkit/commits?author=frostming

Ideally they should be submit the patch, or someone elses should submit it with the patch author set to the correct author. Ping also @techalchemy who has been heavily involved in the pipenv vendoring.

[jayvdb]

ping @frostming , can you confirm you are the author of this?

I've confirmed this patch is needed by pipenv.

Without it, test_uninstall.py::test_normalize_name_uninstall fails at assert "# Inline comment" in contents, with the entire line removed.

Now I understand the patches effect a bit better, this feels like it isnt a complete patch, as that inline comment after an entry probably needs to be removed with the entry in some cases, and it should be up to the caller whether 'dangling' comments should be kept, so some mechanism needs to be added to allow the caller to control this. But that is just a guess, and I'd appreciate feedback from @sdispater about whether this patch is acceptable as-is .

[frostming]

this patch is authored by me to preserve the comment when an entry is removed. It is required by pipenv in the regressiin cases. However it is not always expected
for a general toml document. Cases are user may want to remove the comments together with the line it is bound to. So I just patch it in pipenv vendor but leave it to original creator how to design it.