-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add type hints, logging, pre-commit, exit code docs, and Python API docs #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ambicuity
wants to merge
5
commits into
SUPAIDEAS:main
Choose a base branch
from
ambicuity:fix/cli-enhancements-and-docs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
47c6799
test: Add unit tests for encrypt_pdf function
ambicuity 2605313
Merge upstream/main and resolve conflicts
ambicuity a8e16f3
feat: Add dynamic version flag, handle output collisions, add CONTRIB…
ambicuity 6d9e130
test: Address inline code review feedback from Copilot
ambicuity ae391bf
feat: address issues #36, #37, #40, #41, #42
ambicuity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| repos: | ||
| - repo: https://github.com/pre-commit/pre-commit-hooks | ||
| rev: v4.6.0 | ||
| hooks: | ||
| - id: trailing-whitespace | ||
| - id: end-of-file-fixer | ||
| - id: check-yaml | ||
| - id: check-added-large-files | ||
|
|
||
| - repo: https://github.com/astral-sh/ruff-pre-commit | ||
| rev: v0.4.4 | ||
| hooks: | ||
| # Run the ruff linter | ||
| - id: ruff | ||
| args: [--fix] | ||
| # Run the ruff formatter | ||
| - id: ruff-format |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # Contributing to passifypdf | ||
|
|
||
| First off, thank you for considering contributing to passifypdf! It's people like you that make open source such a great community. | ||
|
|
||
| ## Development Environment Setup | ||
|
|
||
| This project uses `uv` for dependency management and running tools. | ||
|
|
||
| ### 1. Install `uv` | ||
| If you haven't already, install [uv](https://github.com/astral-sh/uv). | ||
| ```bash | ||
| curl -LsSf https://astral.sh/uv/install.sh | sh | ||
| ``` | ||
|
|
||
| ### 2. Fork and Clone | ||
| Fork the repository and clone it to your local machine: | ||
| ```bash | ||
| git clone https://github.com/YOUR_USERNAME/passifypdf.git | ||
| cd passifypdf | ||
| ``` | ||
|
|
||
| ### 3. Install Dependencies | ||
| You can install the dependencies using `uv`. | ||
| ```bash | ||
| uv sync | ||
| ``` | ||
| This will set up the virtual environment with all required run and development dependencies. | ||
|
|
||
| ## Testing | ||
|
|
||
| We use `pytest` for running our test suite. | ||
|
|
||
| To run tests: | ||
| ```bash | ||
| uv run pytest | ||
| ``` | ||
|
|
||
| To run tests with coverage reporting: | ||
| ```bash | ||
| uv run pytest --cov=passifypdf | ||
| ``` | ||
| Please make sure all existing tests pass and write new tests for your newly added code before submitting a Pull Request. | ||
|
|
||
| ## Code Quality | ||
|
|
||
| ### Pre-commit Hooks | ||
|
|
||
| This project uses [`pre-commit`](https://pre-commit.com/) to enforce code quality automatically before each commit. The configuration lives in [`.pre-commit-config.yaml`](.pre-commit-config.yaml) and runs `ruff` (linter + formatter) plus standard file hygiene checks. | ||
|
|
||
| To set it up locally: | ||
|
|
||
| ```bash | ||
| # Install pre-commit (using uv) | ||
| uv run pre-commit install | ||
|
|
||
| # Or using pip | ||
| pip install pre-commit | ||
| pre-commit install | ||
| ``` | ||
|
|
||
| Once installed, hooks run automatically on `git commit`. To run them manually across all files: | ||
|
|
||
| ```bash | ||
| uv run pre-commit run --all-files | ||
| ``` | ||
|
|
||
| ## Code Constraints and Formatting | ||
|
|
||
| Currently, the project strives for PEP 8 compliance. While we may add automated pre-commit formatting hooks in the future, please try to: | ||
| - Follow standard Python styling conventions (use `flake8` or `ruff` to identify issues). | ||
| - Include standard docstrings for all new functions/classes. | ||
| - Ensure your typed logic has appropriate type-hints. | ||
|
|
||
| ## Submitting a Pull Request | ||
|
|
||
| 1. Create a feature branch from your fork: `git checkout -b feature/your-feature-name`. | ||
| 2. Commit your changes: `git commit -m "feat: Add your feature text"`. | ||
| 3. Push to your branch: `git push origin feature/your-feature-name`. | ||
| 4. Submit a Pull Request on the main repository via GitHub. | ||
|
|
||
| Please link any relevant open issues to your Pull Request explicitly (e.g. "Closes #123"). | ||
| CodeRabbit / Copilot AI might review your PR; please address any review feedback they provide. | ||
|
|
||
| Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| #!/bin/bash | ||
|
|
||
| echo "Creating Buf issues..." | ||
| # Bugs | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Special characters and emojis in PDF file paths cause minor complaints" --body "As mentioned in the README, if the user has special characters like an emoji in the PDF file name, the shell gives a minor complain during execution. This should be gracefully handled by encoding the paths correctly or catching the exception and returning a friendly error." --label bug | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Validate if the input file is actually a valid PDF format" --body "Currently, the script might attempt to encrypt non-PDF files if passed with a .pdf extension or no extension. We should use a magic byte checker or a try-catch block during the PdfReader initialization to provide a clear error message that the file is not a valid PDF." --label bug | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: High memory consumption on very large PDF files" --body "The current approach iterates through all pages and adds them to the writer using reader.pages, holding everything in memory. For a multi-gigabyte PDF, this might lead to Out Of Memory (OOM) errors. We should explore optimizing memory usage." --label bug | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Output path collision should prompt before overwriting" --body "If the user specifies an output path that already exists, the script silently overwrites it. It should prompt the user (e.g., 'File exists, overwrite? [y/N]') unless a --force flag is provided." --label bug | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Bug: Handle encrypted PDFs gracefully if passed as input" --body "If the input PDF is already encrypted, pypdf will throw an error when trying to read the pages. The script should catch this specific error and inform the user." --label bug | ||
|
|
||
| echo "Creating Enhancement issues..." | ||
| # Enhancements | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Add an option to decrypt an already protected PDF" --body "It would be useful if passifypdf could also act as a decryption tool if the user provides the correct password and a --decrypt flag." --label enhancement | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Support setting 256-bit AES encryption explicitly" --body "By default, pypdf might use 128-bit encryption based on the version. We should allow advanced users to specify the encryption algorithm explicitly, e.g., --encryption-level 256." --label enhancement | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Add a --quiet or -q flag for silent execution" --body "For scripting and CI/CD pipelines, users might not want the 'Congratulations!' message printed to the console. A --quiet flag should suppress all non-error output." --label enhancement | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Add batch processing support for directories" --body "Allow the input -i to be a directory instead of a file, parsing all .pdf files and creating protected versions in the output directory." --label enhancement | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Enhancement: Display progress bar during encryption" --body "For PDFs with thousands of pages, the script might take a while. Adding a simple progress bar using tqdm or rich would improve the UX considerably." --label enhancement | ||
|
|
||
| echo "Creating Documentation issues..." | ||
| # Documentation | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Add a comprehensive CONTRIBUTING.md guide" --body "We need a standard setup guide for contributors, detailing how to set up the local environment, run tests with pytest, and linting rules." --label documentation | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Update README with GitHub Actions Status Badges" --body "Adding visually appealing badges for CI/CD status, PyPI version, and License will make the project look more professional." --label documentation | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Write a SECURITY.md explaining PDF encryption limits" --body "As noted in the README, PDF encryption should not be relied upon for mission-critical data. A SECURITY.md file detailing the exact cryptographic standards used would help users." --label documentation | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Provide examples of using passifypdf programmatically" --body "While the CLI is documented, other Python developers might want to import encrypt_pdf directly in their code. Add a small Python API section to the README." --label documentation | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Docs: Document expected exit codes and error scenarios" --body "CLI tools should ideally document their exit codes (e.g., 0 for success, 1 for generic error, 2 for missing file). Please add a section in docs/CLI_OPTIONS.md for this." --label documentation | ||
|
|
||
| echo "Creating Good First issues..." | ||
| # Good First Issue | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Refactoring: Migrate argparse to click or typer" --body "The current CLI uses standard library argparse. Migrating to typer or click would provide better automatic help generation, colors, and type enforcement." --label "good first issue" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Feature: Add --version / -v flag to CLI" --body "Currently, there is no quick way to check the installed tool version. A standard --version flag should be added reading the version from pyproject.toml or __init__.py." --label "good first issue" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Refactoring: Use standard logging module instead of print" --body "All console output is currently using print(). We should initialize a basic logging configuration and use logger.info(), logger.error() instead." --label "good first issue" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "CI: Add pre-commit hooks for code formatting" --body "To maintain code quality, we should add a .pre-commit-config.yaml file configuring ruff, black, or flake8 and add instructions on how to set it up." --label "good first issue" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Tests: Add type hints to all test fixtures and functions" --body "The main codebase uses type hints nicely, but the test suite (tests/unittests/test_encryptpdf.py) lacks them. This is a great starting issue to get familiar with the codebase." --label "good first issue" | ||
|
|
||
| echo "Creating Help Wanted issues..." | ||
| # Help Wanted | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Research: Implement hardware-accelerated AES encryption" --body "For large PDFs, standard pypdf encryption might be slow. We are looking for help to profile and possibly integrate hardware-accelerated AES via cryptography or natively." --label "help wanted" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Build: Implement cross-platform native binaries using PyInstaller" --body "Not all users have Python installed. Creating a GitHub Actions pipeline to publish standalone binaries for Windows, macOS, and Linux using PyInstaller would be a fantastic addition." --label "help wanted" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "UX: Create a Streamlit or Gradio based web UI wrapper" --body "Some users prefer graphical interfaces. We need help building a simple, local Web UI wrapper around passifypdf using Streamlit or Gradio." --label "help wanted" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "UX: Integrate into right-click OS context menus" --body "It would be amazing if users could right-click a PDF on Windows/macOS and select 'Protect with passifypdf'. We need contributors familiar with OS registries/automator to add this." --label "help wanted" | ||
| gh issue create --repo SUPAIDEAS/passifypdf --title "Benchmarking: Compare encryption speeds against other CLI tools" --body "We want to know how passifypdf performs against tools like qpdf or pdftk. We need help writing a benchmark suite and documenting the results." --label "help wanted" | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,20 @@ | ||
| import argparse | ||
| from importlib.metadata import version, PackageNotFoundError | ||
|
|
||
|
|
||
| def get_arg_parser() -> argparse.ArgumentParser: | ||
| try: | ||
| __version__ = version("passifypdf") | ||
| except PackageNotFoundError: | ||
| __version__ = "unknown" | ||
|
|
||
| arg_parser = argparse.ArgumentParser( | ||
| description="Encrypt a PDF file with a password of your choice.", | ||
| epilog="For more information, visit: https://github.com/SUPAIDEAS/passifypdf" | ||
| ) | ||
| arg_parser.add_argument("-v", "--version", action="version", version="%(prog)s 1.0") | ||
| arg_parser.add_argument("-v", "--version", action="version", version=f"%(prog)s {__version__}") | ||
| arg_parser.add_argument("-i", "--input", required=True, help="Path to the input PDF file to be encrypted") | ||
| arg_parser.add_argument("-o", "--output", required=True, help="Path where the encrypted PDF file will be saved") | ||
| arg_parser.add_argument("-p", "--passwd", required=True, type=str, help="Password to encrypt the PDF file with") | ||
| arg_parser.add_argument("-f", "--force", action="store_true", help="Overwrite the output file if it already exists without prompting") | ||
| return arg_parser | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing test coverage for the new --version/-v flag functionality. The dynamic version retrieval using importlib.metadata should be tested to ensure it works correctly both when the package is installed and when it's not (PackageNotFoundError case). Consider adding tests that verify the version flag returns the correct version string.