Skip to content

Fix StopIteration in restore_yaml_comments when default_data ends with a comment or blank line#196

Open
gaoflow wants to merge 3 commits into
beetbox:mainfrom
gaoflow:fix/restore-yaml-comments-stop-iteration
Open

Fix StopIteration in restore_yaml_comments when default_data ends with a comment or blank line#196
gaoflow wants to merge 3 commits into
beetbox:mainfrom
gaoflow:fix/restore-yaml-comments-stop-iteration

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 24, 2026

Copy link
Copy Markdown

What

restore_yaml_comments in confuse/yaml_util.py uses a bare next() call inside an inner while True loop to scan past comment or blank lines in default_data. If the file ends on a comment or blank line — which is extremely common, since editors and most YAML files append a trailing newline after the last comment — next() raises StopIteration, which propagates out of the function entirely and crashes any caller.

The most visible symptom: Configuration.dump() raises StopIteration when the application's config_default.yaml has an empty or comment-only last line (issue #148).

Root cause

while True:
    line = next(default_lines)   # raises StopIteration at EOF, no catch
    ...

Fix

Wrap the next() call in a try/except StopIteration, set line = None, and break out of both the inner and outer loops. The trailing comment is silently discarded (there is no following key to attach it to), matching the documented behaviour for orphaned comments.

Tests

Three regression tests added to test/test_yaml.py:

  • test_trailing_comment_does_not_raise — default_data ends with # trailing comment
  • test_trailing_blank_line_does_not_raise — default_data ends with \n\n
  • test_comments_still_restored_before_key — verifies normal comment restoration still works

All 295 existing tests continue to pass.

Closes #148.


This pull request was prepared with the assistance of AI, under my direction and review.

…h comment or blank line

`restore_yaml_comments` used a bare `next()` call inside an inner
`while True` loop to scan past comment/blank lines in `default_data`.
If the file ended on a comment or blank line (very common — editors
often append a trailing newline after the last comment), `next()` raised
`StopIteration`, which propagated out of the function and crashed any
caller, including `Configuration.dump()`.

Fix: catch `StopIteration`, set `line = None`, and break out of both
the inner and outer loops so the orphaned comment is silently discarded
rather than crashing the caller.

Fixes beetbox#148.
@github-actions

Copy link
Copy Markdown

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

@gaoflow

gaoflow commented Jun 26, 2026

Copy link
Copy Markdown
Author

Added a changelog entry under Unreleased for the restore_yaml_comments fix.

Verification:

  • uv run --group lint sphinx-lint --enable all --disable default-role docs/changelog.rst -> passed
  • uv run --group dev pytest test/test_yaml.py -q -> 23 passed
  • git diff --check -> passed

@gaoflow

gaoflow commented Jun 27, 2026

Copy link
Copy Markdown
Author

Fixed the mypy failure in 88d2956 by avoiding assigning None to the line variable in restore_yaml_comments().

Local verification:

uv run --group lint mypy --show-column-numbers --no-error-summary .
uv run --group dev pytest test/test_yaml.py -q
uv run --group lint ruff format --check --diff confuse/yaml_util.py test/test_yaml.py
uv run --group lint ruff check confuse/yaml_util.py test/test_yaml.py
uv run --group lint sphinx-lint --enable all --disable default-role docs/changelog.rst
git diff --check

Note: I could not run the poe wrapper locally because my machine has a broken global Poetry shim on PATH (ModuleNotFoundError: tomlkit), so I ran the underlying mypy/ruff/sphinx-lint commands directly.

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.

Configuration.dump raises StopIteration if the last line is empty.

1 participant