Skip to content

Preserve literal quotes when replacing table strings - #627

Merged
frostming merged 3 commits into
python-poetry:masterfrom
guhou-hvi:fix/preserve-literal-quotes
Nov 3, 2026
Merged

frostming merged 3 commits into
python-poetry:masterfrom
guhou-hvi:fix/preserve-literal-quotes

Conversation

@guhou-hvi

@guhou-hvi guhou-hvi commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Related to #325.

When editing a parsed TOML table, assigning a plain Python string currently replaces single quotes with double quotes: bar = 'baz' becomes bar = "quux" after assigning "quux". This adds formatting changes to an otherwise simple configuration edit.

Preserve single-line literal quoting for replacements that can be represented that way. Values containing an apostrophe, newline, or forbidden control character still use basic strings, which provide the necessary escaping. Explicit tomlkit.string(...) values keep the caller's quote choice. Assignments to new fields, existing multiline-string fields, and array elements keep their existing behavior.

The conversion is shared by document, table, and inline-table assignment. This is needed because tables convert Python strings to TOML items before passing them to the underlying container; checking only the final replacement method would miss the original issue's table example.

Regression tests check root, ordinary, inline, and out-of-order tables, preserving comments, spacing, key order and line endings. They also check the actual values after serialization and reparsing, explicit quote choices, and values requiring escaping. These are additional checks for this change, alongside the project's existing suite.

On October 24, Windows/Python 3.12.2 ran all 1,088 tests successfully, including the 20 regression cases introduced by this PR. All six official pre-commit hooks passed. Supplemental mypy checks reported the same 29 diagnostics on the original baseline and the fix. These results cover the unchanged quoting implementation; the new workflow configuration is checked separately.

The Linux Python 3.9 CI job failed during setup-python, before running tests, because no Python 3.9 build was available for Ubuntu 26.04. This revision incorporates the upstream fix from #681: it replaces only ubuntu-latest / 3.9 with ubuntu-24.04 / 3.9, retaining all 18 test combinations. The PR remains draft pending required CI results for the new commit.

The review revision removes the duplicate plain-string check from Container.__setitem__; _item_for_key retains it. An independent 100-case comparison against the previous PR commit checks the assignment entrances, explicit string identity, parent-dependent conversion, custom encoders, and unchanged state after invalid input.

Seven alternating local performance rounds compare the revised code with both baselines. Against the previous PR commit 87ced370, string assignments take less than 0.13 microseconds less, while integer assignment costs 0.42 microseconds more (4.5%). Against the original master baseline 0518af25, the complete quoting fix adds 1.95–3.49 microseconds per string assignment (13.1–23.5%); parsing 30 fields takes 0.83% longer. This is a formatting-preservation tradeoff, not a general speedup.

Agent Drafting Metadata

  • Agent: OpenAI Codex
  • Model: GPT-6; exact deployment identifier is not exposed in this session.
  • Notes: Codex implemented the quoting change and regression tests, prepared the description, and ran local checks. The CI-only follow-up applies KennySimpson's upstream fix from Keep the Python 3.9 Linux job on Ubuntu 24.04 #681 with its original authorship preserved. Independent Codex agents review semantics and explanation separately. The author reviewed the quoting change and approved this CI follow-up and updated wording.

@Wkhanex Wkhanex left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single-quoted replacement works at root, table, inline-table and out-of-order dotted-key locations. Representable replacements keep their literal quotes; values containing an apostrophe or newline fall back to valid output, and backslashes/double quotes roundtrip correctly.

Checked 87ced370034c with standalone Python 3.12.14 checks (52 passing assertions). I haven't run the full upstream suite.

Comment thread tomlkit/container.py Outdated
def __setitem__(self, key: Key | str, value: Any) -> None:
if key in self:
old_key = next(filter(lambda k: k == key, self._map))
if isinstance(value, str) and not isinstance(value, String):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is run twice, please reduce one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I removed the outer check in Container.__setitem__; the plain-string test now lives in _item_for_key. The existing table and inline-table entry points are unchanged.

On 4158868, all 1,088 tests and the six official pre-commit hooks pass locally. An independent comparison with the previous PR commit also checks explicit String identity, non-string assignments, parent-dependent table conversion, and custom encoders across the assignment entry points. The PR description now records the revised validation and measured assignment costs.

Codex made and checked this revision, with a separate Codex agent reviewing it.

I have kept the PR as a draft while the new CI runs complete. The ubuntu-latest / 3.9 job currently fails in actions/setup-python, before tests run: Python 3.9 is unavailable for its Ubuntu 26.04 runner. Job log.

@guhou-hvi
guhou-hvi marked this pull request as draft October 24, 2026 06:59
@guhou-hvi
guhou-hvi marked this pull request as ready for review November 3, 2026 01:20
@frostming
frostming merged commit e67098d into python-poetry:master Nov 3, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants