Skip to content

fix: apply consistent surrogate-sanitizing read. - #756

Open
StarsExpress wants to merge 12 commits into
mpfaffenberger:mainfrom
StarsExpress:fix-read-text-sanitized
Open

fix: apply consistent surrogate-sanitizing read.#756
StarsExpress wants to merge 12 commits into
mpfaffenberger:mainfrom
StarsExpress:fix-read-text-sanitized

Conversation

@StarsExpress

@StarsExpress StarsExpress commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

With PR #775 refactoring issue #463's part 2 and just merged into main,
PR #756 fixes #463's part 1: align repeated "read file + sanitize surrogates" block.


Repeated code block

with open(file_path, "r", encoding="utf-8", errors="surrogateescape") as f:
    original = f.read()

try:
    original = original.encode("utf-8", errors="surrogatepass").decode("utf-8", errors="replace")

except (UnicodeEncodeError, UnicodeDecodeError):
    pass

Issue #463 cites 4 duplicated call sites (excluding the recently deleted code_puppy/plugins/file_permission_handler.py in 27efe8f):

  • 3 in tools/file_modifications.py_delete_snippet_from_file, _replace_in_file, _write_to_file.
  • 1 in tools/file_operations.py_read_file.

Actual count — 4 + 2 = 6:

  • file_modifications.py has 4th site at _delete_file, and 5th site at _delete_file_async.

All 6 repeated calls are covered by this PR.


Correction

On issue #463's framing: _read_file doesn't actually have "better fallback" as described — it only uses errors="surrogateescape" on open with zero post-read cleanup, making it currently the least robust of 6 call sites.

3-layer fallback _sanitize_string already existed and was used by 13 other call sites; unifying via read_text_sanitized brings _read_file up to that same standard for the first time.


What has changed

  • Moves _sanitize_string from code_puppy/tools/file_operations.py to code_puppy/tools/common.py, since file_operations.py already depends on common.py regarding resolve_path import.

  • Thus, its TestSanitizeString moves from tests/tools/test_file_operations_coverage.py to tests/tools/test_common_full_coverage.py.

  • Defines read_text_sanitized next to atomic_write_text inside code_puppy/tools/common.py.

  • Aligns the aforementioned 8 repeated occurrences with read_text_sanitized.

  • Adds some blank lines in these 8 repeated sites to enhance code readability.

@StarsExpress
StarsExpress marked this pull request as draft August 11, 2026 19:50
1. tools/file_modifications.py — _delete_snippet_from_file, _replace_in_file, _write_to_file.

2. plugins/file_permission_handler/register_callbacks.py.
@StarsExpress
StarsExpress force-pushed the fix-read-text-sanitized branch from c185cc1 to 5d97b0d Compare August 12, 2026 03:30
@StarsExpress
StarsExpress marked this pull request as ready for review August 12, 2026 04:05
@StarsExpress StarsExpress changed the title fix: apply consistent read_text_sanitized. fix: apply consistent surrogate-sanitizing read. Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant