Skip to content

fix(cli): Отвечать ошибкой ввода вместо молчаливого открытия GUI - #46

Merged
zeegin merged 2 commits into
mainfrom
fix/cli-argument-parsing
Sep 17, 2026
Merged

zeegin merged 2 commits into
mainfrom
fix/cli-argument-parsing

Conversation

@zeegin

@zeegin zeegin commented Sep 17, 2026

Copy link
Copy Markdown
Member

Closes #15.

Главное: неверный ввод выглядел как зависание

Разбор принимал единственный шаблон argv[1] == "unpack" and argv[3] == "-tmplts" и на любое отклонение возвращал handled=False. Процесс проваливался в Qt event loop. Сквозной прогон python3 main.py под QT_QPA_PLATFORM=offscreen — до и после:

аргументы было стало
unpack f.efd -tmplts out rc=0, [OK] rc=0, [OK]
unpack f.efd --tmplts out ЗАВИС, вывода нет rc=2 + usage
unpack -tmplts out f.efd ЗАВИС, вывода нет rc=2 + usage
UNPACK f.efd -tmplts out ЗАВИС, вывода нет rc=2 + usage
unpack f.efd ЗАВИС, вывода нет rc=2 + usage
unpack f.efd -tmplts out EXTRA rc=0, [OK] — хвост отброшен rc=2 + usage
unpack --help ЗАВИС, пустое окно rc=0 + помощь
--help rc=0 + помощь rc=0 + помощь

Критерий 4 (unpack --help печатает тот же текст, что --help) проверен diff — выводы идентичны.

GUI-режим не тронут: файл без команды unpack по-прежнему уходит в окно, и на это есть отдельный тест.

Почему не argparse

Issue предлагал argparse. Отказался по двум причинам, обе проверяемые:

  1. allow_abbrev=False не защищает -tmplts. Флаг отключает сокращения только для ---опций; argparse всё равно принял бы -t out и -tmp out. Issue это сам отмечает — значит, флаг пришлось бы сверять строкой в любом случае.
  2. Ошибочный путь argparse — sys.exit(2) в stderr, а проект печатает через инжектируемый output и возвращает CLIResult, не завершая процесс. Ловить SystemExit ради этого — больше кода, чем явный разбор.

Явный разбор занимает 15 строк и даёт ровно нужную семантику. Сокращения -t/-tmp закрыты отдельными тестами.

Схема efd://

Ветка делала parsed.path.lstrip("/"): абсолютный путь становился относительным, netloc терялся вместе с буквой диска, unquote не вызывался. Не работала ни одна реалистичная форма, включая команду прямо из docs/FILE_ASSOCIATION_GUIDE.md. Теперь file:// и efd:// разбираются одним кодом, и тест требует их посимвольного совпадения на пяти именах — ASCII, кириллица, пробел, +, %.

Отдельная ветка для netloc, оканчивающегося на :efd://C:/dir/f.efd это буква диска, а не UNC-хост; без неё путь превратился бы в //C:/dir/f.efd.

eventFilter на macOS больше не гасит событие не-file схемы: при пустом toLocalFile() берётся url().toString(). Это независимый дефект — починка разбора сама по себе клик по efd://-ссылке не оживила бы.

Причина отказа дошла до пользователя

process_file_argument ловил FileValidationError и возвращал None, теряя код. Обе ветки показа причины в main() были при этом мертвы дважды: до них не доходило исключение, а set_input_file сам ловит его и сам показывает локализованный QMessageBox.

Убрал валидацию из process_file_argument (теперь это чистое «URL → путь»), set_input_file вернул bool, мёртвые ветки удалены. Показ ограничен аргументами, похожими на файл или URL — иначе флаги запуска Qt (-platform, --style) давали бы ложное «файл не существует».

format_help_text вынесен в application/help_text.py: помощь нужна и слою CLI, и main, а main импортирует cli, так что обратный импорт замкнул бы цикл — плюс main тянет PyQt5, которого слою CLI не нужно.

Изменение контракта

unpack <file> без -tmplts раньше уводил в GUI. docs/CLI.md называл эту форму «не headless-режимом», но GUI не обещал. Теперь это ошибка ввода: код 2 + usage. Тест test_cli_does_not_handle_incomplete_command переписан с объяснением, поведение задокументировано в docs/CLI.md (новый раздел «Помощь и ошибки ввода»).

Проверил, что строгий разбор не сломает релизный конвейер: все пять смоук-вызовов в build-and-release.yml (строки 191, 238, 258, 321, 381) используют ровно форму unpack <file> -tmplts <dir>.

Тесты

274 → 290. Сняты два xfail(strict=True) на efd://.

Обратные мутации:

мутация результат
вернуть len(rest) >= 3 (хвост отбрасывается) 1 failed
вернуть handled=False на неверном хвосте 10 failed
вернуть lstrip("/") для efd:// 12 failed
290 passed, 3 xfailed
покрытие 86.96%, cli.py — 97%
ruff: All checks passed

Разбор аргументов принимал единственный шаблон и на любое отклонение
возвращал handled=False. Процесс проваливался в Qt event loop: из
терминала поднималось пустое окно, а в CI команда висела до таймаута —
ни сообщения, ни usage, ни кода возврата. Опечатка во флаге, перепутанный
порядок, другой регистр, `unpack --help` — всё это выглядело как
зависание, а не как ошибка ввода.

Теперь команда unpack всегда обрабатывается в консоли: при неверном
синтаксисе печатается помощь и возвращается код 2. GUI-режим не тронут —
файл без команды unpack по-прежнему уходит в окно.

- Флаг сверяется строкой, а не argparse: allow_abbrev=False отключает
  сокращения только для --опций, и argparse принял бы `-t out` как
  -tmplts. Проверено на 3.9.
- Хвост после каталога отклоняется: `unpack a.efd -tmplts out b.efd`
  отвечал [OK] с кодом 0, обработав только первый файл.
- --help и -h ищутся по всему argv, а не только в argv[1]. Полный
  argparse на верхнем уровне не подходит: main намеренно пропускает
  нераспознанные аргументы в Qt.
- Схема efd:// приведена к разбору file://. Ветка делала
  parsed.path.lstrip("/"), поэтому абсолютный путь становился
  относительным, netloc терялся вместе с буквой диска, а percent-encoding
  не раскрывался. Не работала ни одна реалистичная форма, включая команду
  прямо из FILE_ASSOCIATION_GUIDE.
- eventFilter на macOS больше не гасит событие не-file схемы: при пустом
  toLocalFile() берётся url().toString().
- process_file_argument больше не валидирует и не теряет код ошибки.
  Причину показывает set_input_file, который уже ловит исключение и уже
  показывает локализованный QMessageBox; две мёртвые ветки показа ошибки
  в main() удалены. Сообщение показывается только для аргументов, похожих
  на файл или URL, иначе посторонние флаги Qt давали бы ложные отказы.

format_help_text вынесен в application/help_text.py: помощь нужна и слою
CLI, и main, а обратный импорт замкнул бы цикл.

Контракт `unpack <file>` без -tmplts изменён намеренно: docs/CLI.md
называл эту форму «не headless-режимом», но GUI не обещал. Тест на
прежнее поведение переписан, поведение задокументировано.

Тесты: 274 -> 290, сняты два xfail(strict=True).

Closes #15
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-17T12:46:01.496208Z 96dd511 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

URL всегда несёт прямые слэши, а str(Path) на Windows — обратные:
C:/Users/x/a.efd и C:\\Users\\x\\a.efd указывают на один файл, но как
строки не равны. Разделители приводит к системным os.path.abspath уже
внутри валидатора, так что на поведение это не влияло — только на
строгость самих тестов.

Заодно два теста на Windows собирали не ту форму URL. as_posix() там
начинается с буквы диска, поэтому efd:// + as_posix() давало форму
с authority (efd://C:/...) и уходило в ветку разбора буквы диска,
а не в форму с тремя слэшами, которую тест заявлял. Теперь URL
собирается из as_uri() и одинаков на всех платформах.

Тест на localhost по той же причине собирал невалидный netloc
"localhostC:" — хост вставляется в готовый file://-URL.
@zeegin
zeegin merged commit 2d67ed1 into main Sep 17, 2026
3 checks passed
@zeegin
zeegin deleted the fix/cli-argument-parsing branch September 17, 2026 13:20
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.

Разбор CLI-аргументов позиционный: неверный ввод молча открывает GUI

1 participant