refactor(files): extract _load_file_for_editing helper
Step 11/12 of docs/superpowers/plans/2026-05-17-files-overlay-rewrite.md.
overlay_file_content and overlay_file_edit_page used to repeat the
same prelude: load the files-overlay, resolve sub_path safely, confirm
it's a regular file, attempt a UTF-8 read with the is_editable sniff
+ UnicodeDecodeError-on-tail fallback. The bodies have diverged in
two places (the binary branch differs — content endpoint returns 415,
edit page renders the binary template) but the prelude is identical.
Extract _load_file_for_editing(overlay_id, sub_path, user). Returns
either a Flask Response (400/404/403/500) or a 5-tuple
(overlay, target, content_or_None, is_binary, byte_count). Both
routes call it and translate the tuple into their output shape:
* overlay_file_content: 415 when is_binary, else jsonify text
* overlay_file_edit_page: render binary template when is_binary,
else render text template
The previous _render_binary_editor helper folds back into the route
since it was only called from there and benefits from inlining now
that the prelude is shared.
is_binary in the tuple is True both when is_editable() said no up
front (8-KiB sniff) AND when the tail-of-file read raised
UnicodeDecodeError — collapsing what was a duplicated try/except
branch in each route.
No behavior change. pytest still 580 passed, 1 skipped, 3 deselected.
Step 12 (next) audits /files/content callers and likely deletes the
endpoint since Phase B removed its only caller (legacy
openEditorForFile via files-overlay.js); the shared helper would
then collapse to single-caller status but remains useful for
grouping the prelude into a named concept.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
ddf03c6fb8
commit
3facc323b6
1 changed files with 63 additions and 61 deletions
|
|
@ -201,6 +201,48 @@ def server_files_fragment(server_id: int):
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _load_file_for_editing(overlay_id: int, sub_path: str, user):
|
||||||
|
"""Shared prelude for overlay_file_content + overlay_file_edit_page:
|
||||||
|
load the files-overlay, resolve sub_path safely, confirm it's a
|
||||||
|
regular file, attempt a UTF-8 read.
|
||||||
|
|
||||||
|
Returns either:
|
||||||
|
* a 5-tuple (overlay, target, content, is_binary, byte_count) on
|
||||||
|
success — content is the UTF-8 text in text mode, None in
|
||||||
|
binary mode; byte_count is encoded text length or st_size
|
||||||
|
* a Flask Response on any failure: 400 (path traversal), 404
|
||||||
|
(missing or non-file), 403 (system overlay, non-admin), 500
|
||||||
|
(OS read error)
|
||||||
|
|
||||||
|
is_binary is True both when is_editable(target) said no up front
|
||||||
|
(8-KiB sniff) and when the tail of the file failed UTF-8 decode
|
||||||
|
even though the sniff passed."""
|
||||||
|
result = _load_files_overlay(overlay_id, user)
|
||||||
|
if isinstance(result, Response):
|
||||||
|
return result
|
||||||
|
overlay = result
|
||||||
|
|
||||||
|
try:
|
||||||
|
target = safe_resolve_for_listing(overlay.path, sub_path)
|
||||||
|
except ValueError:
|
||||||
|
return Response("invalid path", status=400)
|
||||||
|
|
||||||
|
if not target.exists() or not target.is_file():
|
||||||
|
return Response(status=404)
|
||||||
|
|
||||||
|
if not is_editable(target):
|
||||||
|
return overlay, target, None, True, target.stat().st_size
|
||||||
|
|
||||||
|
try:
|
||||||
|
content = target.read_text(encoding="utf-8")
|
||||||
|
except OSError:
|
||||||
|
return Response("read failed", status=500)
|
||||||
|
except UnicodeDecodeError:
|
||||||
|
return overlay, target, None, True, target.stat().st_size
|
||||||
|
|
||||||
|
return overlay, target, content, False, len(content.encode("utf-8"))
|
||||||
|
|
||||||
|
|
||||||
@bp.get("/overlays/<int:overlay_id>/files/content")
|
@bp.get("/overlays/<int:overlay_id>/files/content")
|
||||||
@require_login
|
@require_login
|
||||||
def overlay_file_content(overlay_id: int):
|
def overlay_file_content(overlay_id: int):
|
||||||
|
|
@ -209,29 +251,13 @@ def overlay_file_content(overlay_id: int):
|
||||||
assert user is not None
|
assert user is not None
|
||||||
sub_path = request.args.get("path", "")
|
sub_path = request.args.get("path", "")
|
||||||
|
|
||||||
result = _load_files_overlay(overlay_id, user)
|
result = _load_file_for_editing(overlay_id, sub_path, user)
|
||||||
if isinstance(result, Response):
|
if isinstance(result, Response):
|
||||||
return result
|
return result
|
||||||
overlay = result
|
_overlay, _target, content, is_binary, _byte_count = result
|
||||||
|
|
||||||
try:
|
if is_binary:
|
||||||
target = safe_resolve_for_listing(overlay.path, sub_path)
|
|
||||||
except ValueError:
|
|
||||||
return Response("invalid path", status=400)
|
|
||||||
|
|
||||||
if not target.exists() or not target.is_file():
|
|
||||||
return Response(status=404)
|
|
||||||
if not is_editable(target):
|
|
||||||
return Response("not editable", status=415)
|
return Response("not editable", status=415)
|
||||||
|
|
||||||
try:
|
|
||||||
content = target.read_text(encoding="utf-8")
|
|
||||||
except OSError:
|
|
||||||
return Response("read failed", status=500)
|
|
||||||
except UnicodeDecodeError:
|
|
||||||
# is_editable sniffed only the first 8 KiB; the tail can still fail.
|
|
||||||
return Response("not editable", status=415)
|
|
||||||
|
|
||||||
return jsonify({"path": sub_path, "content": content})
|
return jsonify({"path": sub_path, "content": content})
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -240,42 +266,37 @@ def overlay_file_content(overlay_id: int):
|
||||||
def overlay_file_edit_page(overlay_id: int):
|
def overlay_file_edit_page(overlay_id: int):
|
||||||
"""Server-rendered editor page. Renders full-page by default or as a
|
"""Server-rendered editor page. Renders full-page by default or as a
|
||||||
layoutless modal fragment when the HX-Modal header is set (see the
|
layoutless modal fragment when the HX-Modal header is set (see the
|
||||||
inject_base_layout context processor in app.py)."""
|
inject_base_layout context processor in app.py). Non-editable files
|
||||||
|
render the binary-replace branch instead of returning 415."""
|
||||||
user = current_user()
|
user = current_user()
|
||||||
assert user is not None
|
assert user is not None
|
||||||
sub_path = request.args.get("path", "")
|
sub_path = request.args.get("path", "")
|
||||||
|
|
||||||
result = _load_files_overlay(overlay_id, user)
|
result = _load_file_for_editing(overlay_id, sub_path, user)
|
||||||
if isinstance(result, Response):
|
if isinstance(result, Response):
|
||||||
return result
|
return result
|
||||||
overlay = result
|
overlay, target, content, is_binary, byte_count = result
|
||||||
|
|
||||||
try:
|
if is_binary:
|
||||||
target = safe_resolve_for_listing(overlay.path, sub_path)
|
mime, _enc = mimetypes.guess_type(target.name)
|
||||||
except ValueError:
|
return render_template(
|
||||||
return Response("invalid path", status=400)
|
"overlay_file_editor.html",
|
||||||
|
overlay=overlay,
|
||||||
if not target.exists() or not target.is_file():
|
rel_path=sub_path,
|
||||||
return Response(status=404)
|
content="",
|
||||||
|
byte_count=byte_count,
|
||||||
if not is_editable(target):
|
is_new=False,
|
||||||
return _render_binary_editor(overlay, sub_path, target)
|
is_binary=True,
|
||||||
|
at_folder="",
|
||||||
try:
|
mime_type=mime or "application/octet-stream",
|
||||||
content = target.read_text(encoding="utf-8")
|
)
|
||||||
except OSError:
|
|
||||||
return Response("read failed", status=500)
|
|
||||||
except UnicodeDecodeError:
|
|
||||||
# is_editable's 8-KiB sniff said yes but the tail was binary.
|
|
||||||
# Fall back to the binary template path.
|
|
||||||
return _render_binary_editor(overlay, sub_path, target)
|
|
||||||
|
|
||||||
return render_template(
|
return render_template(
|
||||||
"overlay_file_editor.html",
|
"overlay_file_editor.html",
|
||||||
overlay=overlay,
|
overlay=overlay,
|
||||||
rel_path=sub_path,
|
rel_path=sub_path,
|
||||||
content=content,
|
content=content,
|
||||||
byte_count=len(content.encode("utf-8")),
|
byte_count=byte_count,
|
||||||
is_new=False,
|
is_new=False,
|
||||||
is_binary=False,
|
is_binary=False,
|
||||||
at_folder="",
|
at_folder="",
|
||||||
|
|
@ -283,25 +304,6 @@ def overlay_file_edit_page(overlay_id: int):
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
def _render_binary_editor(overlay, sub_path, target):
|
|
||||||
"""Render overlay_file_editor.html in binary-replace mode. Used by
|
|
||||||
overlay_file_edit_page when the target file isn't a UTF-8 text file.
|
|
||||||
The template hides the CM6 textarea + language dropdown and shows a
|
|
||||||
Replace zone + Download link; the save button reads 'Replace'."""
|
|
||||||
mime, _enc = mimetypes.guess_type(target.name)
|
|
||||||
return render_template(
|
|
||||||
"overlay_file_editor.html",
|
|
||||||
overlay=overlay,
|
|
||||||
rel_path=sub_path,
|
|
||||||
content="",
|
|
||||||
byte_count=target.stat().st_size,
|
|
||||||
is_new=False,
|
|
||||||
is_binary=True,
|
|
||||||
at_folder="",
|
|
||||||
mime_type=mime or "application/octet-stream",
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
@bp.get("/overlays/<int:overlay_id>/files/new")
|
@bp.get("/overlays/<int:overlay_id>/files/new")
|
||||||
@require_login
|
@require_login
|
||||||
def overlay_file_new_page(overlay_id: int):
|
def overlay_file_new_page(overlay_id: int):
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue