feat: migrate CLI from argparse to typer#49
Conversation
- Rewrote passifypdf/cli.py using Typer with typed parameters, automatic input-file validation (exists=True), colored output via Rich, and --force/-f flag for output collision handling - Added --version/-v flag via eager callback with importlib.metadata - Updated passifypdf/encryptpdf.py: main() now delegates to typer app; encrypt_pdf() core logic is unchanged for backward compatibility - Added 10 unit tests in tests/unittests/test_cli.py using typer.testing.CliRunner covering: help, version, success, force bypass, user prompt accept/decline, missing args, nonexistent input, and encrypt_pdf exception propagation - Added typer and typing_extensions to pyproject.toml dependencies
There was a problem hiding this comment.
Pull request overview
This PR migrates the CLI from Python's standard library argparse to Typer, aiming to provide richer help output, automatic type enforcement, and colored terminal output. However, the implementation has several critical issues that need to be addressed before merging.
Changes:
- Replaced argparse-based CLI with Typer implementation providing type-safe options and rich terminal output
- Updated entry point in
encryptpdf.pyto delegate to Typer app - Added typer and typing-extensions dependencies
- Created comprehensive unit tests for the new CLI
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| passifypdf/cli.py | Complete rewrite from argparse to Typer with enhanced features like file existence validation and overwrite prompting |
| passifypdf/encryptpdf.py | Updated main() entry point to delegate to Typer app instead of argparse |
| pyproject.toml | Added typer[standard] and typing-extensions dependencies |
| tests/unittests/test_cli.py | New comprehensive test suite covering CLI functionality including happy path, error cases, and user interaction scenarios |
| uv.lock | Added uv package manager lock file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.info( | ||
| "Congratulations!\nPDF file encrypted successfully and saved as '%s'", | ||
| output, | ||
| ) | ||
| except Exception as e: | ||
| logger.error("Error: %s", e) | ||
| raise typer.Exit(code=1) |
There was a problem hiding this comment.
The CLI uses logger.info() and logger.error() but logging is never configured, so these messages won't be visible to users. The old implementation used print() to display messages to stdout/stderr. Either switch back to using typer.echo() and typer.secho() (which are Typer's recommended output methods), or add logging configuration. For a CLI tool, typer.echo() is more appropriate for user-facing messages.
| """Unit tests for the Typer-based CLI module.""" | ||
|
|
||
| from pathlib import Path | ||
| from typing import Generator |
There was a problem hiding this comment.
The Generator type is imported but never used in the test file. Remove this unused import.
| from typing import Generator |
| raise typer.Exit() | ||
|
|
||
|
|
||
| @app.command() |
There was a problem hiding this comment.
Using @app.command() creates a subcommand, which means users might need to run passifypdf encrypt --input ... instead of passifypdf --input .... This breaks backward compatibility with the documented usage in README.md (line 40) which shows direct option usage without a subcommand. To maintain backward compatibility and provide a simpler CLI, use @app.callback() instead of @app.command(), or use app.command(name="") to make it the default command that doesn't require typing "encrypt".
| @app.command() | |
| @app.callback() |
Summary
Closes #38
Migrates the CLI from the Python standard-library
argparseto Typer, providing richer help output (via Rich), automatic type enforcement, and coloured terminal output.Changes
passifypdf/cli.py- fully rewritten with Typer: typedAnnotatedparameters,exists=Truevalidation for the input file,--force/-fflag,--version/-vvia eager callback, and aget_typer_app()helperpassifypdf/encryptpdf.py-main()now delegates toapp()(the Typer instance);encrypt_pdf()is unchangedtests/unittests/test_cli.py- 10 new unit tests usingtyper.testing.CliRunner(help, version, success, force bypass, user prompt accept/decline, missing args, non-existent input, error propagation)pyproject.toml- addedtyper[standard]>=0.12.0andtyping_extensions>=4.0Testing
All 14 tests pass.