Skip to content

Commit ebb0ef2

Browse files
Copilotnathanjmcdougall
Copilot
and
nathanjmcdougall
authored
Fix TOML comment stripping in extend_list and remove_from_list (#1591)
* Initial plan * Fix TOML comment stripping in extend_list and remove_from_list Use in-place append/remove on tomlkit Array objects instead of creating plain Python lists, which strips comments and formatting. Fixes #884 Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/032e12ce-4fc2-4e64-87e1-f3914b7fcf59 Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> * Use insert() instead of append() to satisfy basedpyright type checker Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/032e12ce-4fc2-4e64-87e1-f3914b7fcf59 Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> * Fix TestAddAuthor.test_append: unwrap tomlkit Array to plain dicts in add_author The in-place Array modification in extend_list now preserves the tomlkit Array type. The add_author delete/re-set dance needs plain Python dicts to trigger tomlkit's format conversion to [[project.authors]]. Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/6a19efab-dd8b-4ba2-ae5e-1f9670293228 Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> * Fix static check failures: ty type error in author.py and AGENTS.md table alignment Agent-Logs-Url: https://github.com/usethis-python/usethis-python/sessions/109d7daa-f84f-425a-b82b-ebb5e8491acf Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nathanjmcdougall <18602289+nathanjmcdougall@users.noreply.github.com> Co-authored-by: Nathan McDougall <nathan.j.mcdougall@gmail.com>
1 parent 23296c2 commit ebb0ef2

5 files changed

Lines changed: 103 additions & 5 deletions

File tree

‎src/usethis/_core/author.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Author metadata management for pyproject.toml."""
22

3+
from collections.abc import Mapping
4+
35
from usethis._console import tick_print
46
from usethis._file.pyproject_toml.io_ import PyprojectTOMLManager
57
from usethis._init import ensure_pyproject_toml
@@ -29,7 +31,12 @@ def add_author(
2931

3032
# Moving the authors list to the end of the project table to avoid a bug in tomlkit
3133
# Suspected to be similar to this https://github.com/python-poetry/tomlkit/issues/381
32-
full = PyprojectTOMLManager()[["project", "authors"]]
34+
full_raw = PyprojectTOMLManager()[["project", "authors"]]
35+
assert isinstance(full_raw, list)
36+
full = []
37+
for item in full_raw:
38+
assert isinstance(item, Mapping)
39+
full.append(dict(item))
3340
del PyprojectTOMLManager()[["project", "authors"]]
3441
PyprojectTOMLManager()[["project", "authors"]] = full
3542
else:

‎src/usethis/_file/toml/io_.py‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,8 @@ def extend_list(self, *, keys: Sequence[Key], values: Sequence[Any]) -> None:
326326
)
327327
raise TOMLValueInvalidError(msg) from None
328328
assert isinstance(d, list)
329-
p_parent[keys[-1]] = d + list(values)
329+
for value in values:
330+
d.insert(len(d), value)
330331

331332
self.commit(toml_document)
332333

@@ -365,8 +366,9 @@ def remove_from_list(self, *, keys: Sequence[Key], values: Collection[Any]) -> N
365366
return
366367
assert isinstance(p, list)
367368

368-
new_values = [value for value in p if value not in values]
369-
p_parent[keys[-1]] = new_values
369+
for value in list(values):
370+
while value in p:
371+
p.remove(value)
370372

371373
self.commit(toml_document)
372374

‎tests/usethis/_core/test_author.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ def test_append(self, tmp_path: Path):
5858
assert (tmp_path / "pyproject.toml").read_text() == (
5959
"""\
6060
61-
6261
[[project.authors]]
6362
name = "First Contributor"
6463
[[project.authors]]

‎tests/usethis/_file/toml/test_toml_io_.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,35 @@ def relative_path(self) -> Path:
663663
):
664664
manager.extend_list(keys=["key"], values=["new_value"])
665665

666+
def test_preserves_comments(self, tmp_path: Path) -> None:
667+
# https://github.com/usethis-python/usethis-python/issues/884
668+
# Arrange
669+
class MyTOMLFileManager(TOMLFileManager):
670+
@property
671+
@override
672+
def relative_path(self) -> Path:
673+
return Path("myfile.toml")
674+
675+
(tmp_path / "myfile.toml").write_text(
676+
"""\
677+
key = [
678+
"A", # Comment for A.
679+
"B", # Comment for B.
680+
]
681+
"""
682+
)
683+
684+
with change_cwd(tmp_path), MyTOMLFileManager() as manager:
685+
manager.read_file()
686+
687+
# Act
688+
manager.extend_list(keys=["key"], values=["C"])
689+
690+
# Assert
691+
contents = (tmp_path / "myfile.toml").read_text()
692+
assert "# Comment for A." in contents
693+
assert "# Comment for B." in contents
694+
666695
class TestRemoveFromList:
667696
def test_inplace_modifications(self, tmp_path: Path) -> None:
668697
# Arrange
@@ -777,3 +806,33 @@ def relative_path(self) -> Path:
777806

778807
# Assert
779808
assert manager._content == {"key": "value"}
809+
810+
def test_preserves_comments(self, tmp_path: Path) -> None:
811+
# https://github.com/usethis-python/usethis-python/issues/884
812+
# Arrange
813+
class MyTOMLFileManager(TOMLFileManager):
814+
@property
815+
@override
816+
def relative_path(self) -> Path:
817+
return Path("myfile.toml")
818+
819+
(tmp_path / "myfile.toml").write_text(
820+
"""\
821+
key = [
822+
"A", # Comment for A.
823+
"B", # Comment for B.
824+
"C", # Comment for C.
825+
]
826+
"""
827+
)
828+
829+
with change_cwd(tmp_path), MyTOMLFileManager() as manager:
830+
manager.read_file()
831+
832+
# Act
833+
manager.remove_from_list(keys=["key"], values=["B"])
834+
835+
# Assert
836+
contents = (tmp_path / "myfile.toml").read_text()
837+
assert "# Comment for A." in contents
838+
assert "# Comment for C." in contents

‎tests/usethis/_tool/impl/base/test_ruff.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,37 @@ def test_group_replaces_specific(self, tmp_path: Path):
329329
assert RuffTool().ignored_rules() == ["TC"]
330330
assert result
331331

332+
def test_preserves_comments(self, tmp_path: Path):
333+
# https://github.com/usethis-python/usethis-python/issues/884
334+
# Arrange
335+
(tmp_path / "ruff.toml").write_text(
336+
"""\
337+
lint.ignore = [
338+
"ANN401", # This is too strict for dunder methods.
339+
"B023", # Prevents using df.loc[lambda _: ...]; too many false positives.
340+
"B024", # This is controversial, ABC's don't always need methods.
341+
"C408", # This is controversial, calls to `dict` can be more idiomatic than {}.
342+
]
343+
"""
344+
)
345+
346+
# Act
347+
with change_cwd(tmp_path), RuffTOMLManager():
348+
RuffTool().ignore_rules(["ERA001"])
349+
350+
# Assert
351+
contents = (tmp_path / "ruff.toml").read_text()
352+
assert "# This is too strict for dunder methods." in contents
353+
assert "# Prevents using df.loc[lambda _: ..." in contents
354+
assert (
355+
"# This is controversial, ABC's don't always need methods." in contents
356+
)
357+
assert (
358+
"# This is controversial, calls to `dict` can be more idiomatic than {}."
359+
in contents
360+
)
361+
assert '"ERA001"' in contents
362+
332363
class TestIsLinterUsed:
333364
def test_neither_subtool_has_config_assume_both_used(self, tmp_path: Path):
334365
# Act

0 commit comments

Comments
 (0)