Fix pyproject.toml formatting when appending alembic config #1684
Conversation
- Modified _append_template to ensure proper TOML section spacing - Added special handling for .toml files to insert two newlines between sections - Added tests to prevent regression - Fixes sqlalchemy#1679
There was a problem hiding this comment.
we were ready to merge a quick one liner for this over at #1680 , which started out as more like this PR. I like that this PR has tests. If we are really going to try to check for a .toml file that already has trailing whitespace, vs. one that does not, it would need to be done non-intrusively and the append of the whitespace should be down with the template write.
| # TOML convention: ensure 2 newlines between top-level sections | ||
| # The template should start with [tool.alembic], so we need proper spacing | ||
| if existing_content: | ||
| f.write('\n\n') |
There was a problem hiding this comment.
OK but what if the template_to_file command fails? now we have two newlines that will keep getting bumped each time (although I guess the strip() and rewrite-the-whole-file approach fixes that, but I dont want to rewrite the whole file). Id rather this be all within one file.open() and once we have the output from the template
in #1680 we add the two newlines in the single open
| existing_content = f.read() | ||
|
|
||
| # Check if we need to add newlines before appending | ||
| existing_content = existing_content.rstrip() |
There was a problem hiding this comment.
this is not really a "check", this just truncates what was there so that it can add two newlines unconditionally
| if output_path.suffix.lower() == '.toml' and output_path.exists(): | ||
| # Read existing content | ||
| with open(output_path, 'r', encoding=self.output_encoding) as f: | ||
| existing_content = f.read() |
There was a problem hiding this comment.
we've read the content here. Can we just look to see if it already ends with "\n\n" ? that's what we did originally at #1680 why rewrite the file
|
|
||
| # TOML convention: ensure 2 newlines between top-level sections | ||
| # The template should start with [tool.alembic], so we need proper spacing | ||
| if existing_content: |
There was a problem hiding this comment.
why did we open and rewrite the file if no existing content?
| from alembic.testing import TestBase | ||
|
|
||
|
|
||
| class TestPyprojectTemplate(TestBase): |
There was a problem hiding this comment.
good test but this should be in tests/test_command.py->CommandLineTest->test_init_file_pyproject_formatting , it should have four cases: one where there's no pyproject.toml, one where there's an empty pyproject.toml, one where there's an existing pyproject.toml with no trailing whitespace (maybe even a fifth test, where there's one trailing newline, not two), one where there's a pyproject.toml with existing trailing whitespace.
Description
This PR fixes issue #1679 where
alembic init --template pyprojectwould concatenate sections without proper spacing when appending to an existingpyproject.tomlfile.Problem: The
[tool.alembic]section was appended directly after the last line of the existing content, resulting in invalid TOML:[tool.mypy]
python_version = "3.10"[tool.alembic]
Solution: Modified
_append_templatemethod inScriptDirectoryto detect TOML files and ensure proper spacing (2 newlines) between top-level sections.Result: Proper TOML formatting is maintained: