feat: add type hints, logging, pre-commit, exit code docs, and Python API docs by ambicuity · Pull Request #48 · SUPAIDEAS/passifypdf

ambicuity · 2026-02-20T15:38:00Z

Summary

This PR addresses the following open issues:

Closes Tests: Add type hints to all test fixtures and functions #42 — Tests: Added -> None return type hints to all test methods and fixtures in test_encryptpdf.py and test_cli.py.
Closes CI: Add pre-commit hooks for code formatting #41 — CI: Added .pre-commit-config.yaml with ruff (linter + formatter) and standard pre-commit-hooks (trailing whitespace, EOF fixer, YAML checker). Updated CONTRIBUTING.md with pre-commit setup instructions.
Closes Refactoring: Use standard logging module instead of print #40 — Refactoring: Replaced all print() calls in encryptpdf.py with Python's standard logging module (logger.info / logger.error). Updated test mocks to patch passifypdf.encryptpdf.logger instead of builtins.print.
Closes Docs: Document expected exit codes and error scenarios #37 — Docs: Added an Exit Codes section to docs/CLI_OPTIONS.md documenting exit code 0 (success) and 1 (failure). Also added the missing --force flag to the options table and updated the stale poetry run example to use uv.
Closes Docs: Provide examples of using passifypdf programmatically #36 — Docs: Added a Programmatic Usage (Python API) section to README.md showing how to import and call encrypt_pdf directly. Also updated install/usage instructions from poetry to uv, and fixed several typos (nay → any, ingore → ignore, etc.).

Testing

All 15 existing tests pass with 89% code coverage:

15 passed in 1.20s
passifypdf/__init__.py    0      0   100%
passifypdf/cli.py        14      0   100%
passifypdf/encryptpdf.py 41      6    85%
TOTAL                    55      6    89%

Add unit tests for encrypt_pdf function to verify functionality, including output file creation, encryption status, decryption with correct password, page count preservation, and handling of invalid input.

…UTING.md

Copilot

Pull request overview

This PR adds three key features to address reported issues: a dynamic version flag that reads from package metadata, an interactive overwrite prompt with a --force bypass option, and comprehensive contributor documentation.

Changes:

Added dynamic --version/-v flag using importlib.metadata to retrieve version from installed package
Implemented file overwrite confirmation prompt with --force flag to bypass
Created CONTRIBUTING.md with setup instructions, testing guidelines, and PR submission process

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
passifypdf/cli.py	Added dynamic version retrieval and --force flag argument
passifypdf/encryptpdf.py	Added output file existence check with interactive overwrite prompt
tests/unittests/test_encryptpdf.py	Added comprehensive tests for encryption function and overwrite prompt behavior; renamed test class for clarity
CONTRIBUTING.md	Added contributor guide with uv setup, testing, and PR submission instructions
poetry.lock	Generated lock file with dependency specifications

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T15:43:11Z

+    def tearDown(self):
+        """Clean up test files."""
+        if os.path.exists(self.output_pdf):
+            os.remove(self.output_pdf)
+        if os.path.exists(self.temp_dir):
+            os.rmdir(self.temp_dir)


The test tearDown may fail if the temp_dir is not empty. The code removes the output_pdf file but then tries to remove the temp_dir with os.rmdir(), which only works for empty directories. If there are any other files created in temp_dir during test execution, this will raise an OSError. Consider using shutil.rmtree(self.temp_dir) instead, or check if the directory is empty before attempting removal.

Copilot · 2026-02-20T15:43:11Z

+
+        self.assertEqual(result, 0)
+        mock_encrypt_pdf.assert_not_called()
+        mock_print.assert_called_with("Operation cancelled.")


Missing test coverage for the case when the output file does NOT exist. The overwrite prompt should not be shown when the output file doesn't exist, and encryption should proceed directly. Consider adding a test case like "test_main_without_force_flag_file_not_exists" to verify this behavior is correct.

Suggested change

mock_print.assert_called_with("Operation cancelled.")

mock_print.assert_called_with("Operation cancelled.")

@patch('passifypdf.encryptpdf.get_arg_parser')

@patch('passifypdf.encryptpdf.encrypt_pdf')

@patch('passifypdf.encryptpdf.Path')

@patch('builtins.print')

@patch('builtins.input')

def test_main_without_force_flag_file_not_exists(self, mock_input, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser):

"""Test main() without --force flag when output file does not exist."""

mock_parser_instance = mock_arg_parser.return_value

mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': False})()

mock_parser_instance.parse_args.return_value = mock_args

mock_path_instance = mock_path_cls.return_value

mock_path_instance.exists.return_value = False

from passifypdf.encryptpdf import main

result = main()

self.assertEqual(result, 0)

mock_encrypt_pdf.assert_called_with('in.pdf', 'out.pdf', 'pass')

mock_input.assert_not_called()

Copilot · 2026-02-20T15:43:12Z


    try:
+        output_path = Path(args.output)
+        if output_path.exists() and not getattr(args, 'force', False):


The use of getattr(args, 'force', False) is unnecessarily defensive since the --force argument is now explicitly defined in the argument parser with action="store_true", which guarantees the 'force' attribute will always exist on args (defaulting to False). Using args.force directly would be clearer and more consistent with how other arguments like args.input, args.output, and args.passwd are accessed in the same function.

Suggested change

if output_path.exists() and not getattr(args, 'force', False):

if output_path.exists() and not args.force:

Copilot · 2026-02-20T15:43:12Z

+    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__}")


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.

Copilot · 2026-02-20T15:43:12Z

+    @patch('passifypdf.encryptpdf.get_arg_parser')
+    @patch('passifypdf.encryptpdf.encrypt_pdf')
+    @patch('passifypdf.encryptpdf.Path')
+    @patch('builtins.print')
+    def test_main_with_force_flag(self, mock_print, mock_path_cls, mock_encrypt_pdf, mock_arg_parser):
+        """Test main() with --force flag when output file exists."""
+        mock_parser_instance = mock_arg_parser.return_value
+        mock_args = type('Args', (), {'input': 'in.pdf', 'output': 'out.pdf', 'passwd': 'pass', 'force': True})()
+        mock_parser_instance.parse_args.return_value = mock_args
+
+        mock_path_instance = mock_path_cls.return_value
+        mock_path_instance.exists.return_value = True
+
+        from passifypdf.encryptpdf import main
+        result = main()
+
+        self.assertEqual(result, 0)
+        mock_encrypt_pdf.assert_called_with('in.pdf', 'out.pdf', 'pass')


In the test mocks, Path is mocked at the class level, but the tests don't verify that Path was called with the correct argument (args.output). Consider adding an assertion like mock_path_cls.assert_called_with('out.pdf') to ensure the output path is being checked correctly, similar to how test_encrypt_pdf verifies Path is called with the input path.

…EAS#41, SUPAIDEAS#42 - feat(tests): add -> None return type hints to all test methods and fixtures in test_encryptpdf.py and test_cli.py (closes SUPAIDEAS#42) - feat(ci): add .pre-commit-config.yaml with ruff and pre-commit-hooks; update CONTRIBUTING.md with pre-commit setup instructions (closes SUPAIDEAS#41) - refactor: replace print() calls with standard logging module in encryptpdf.py; update test mocks to patch logger instead of builtins.print (closes SUPAIDEAS#40) - docs: add Exit Codes section and --force flag to docs/CLI_OPTIONS.md; fix outdated poetry references to use uv (closes SUPAIDEAS#37) - docs: add Programmatic Usage (Python API) section to README.md; update install/usage instructions from poetry to uv; fix typos ('nay' -> 'any', 'complain' -> 'complaint', etc.) (closes SUPAIDEAS#36)

ambicuity and others added 3 commits October 23, 2025 01:44

test: Add unit tests for encrypt_pdf function

47c6799

Add unit tests for encrypt_pdf function to verify functionality, including output file creation, encryption status, decryption with correct password, page count preservation, and handling of invalid input.

Merge upstream/main and resolve conflicts

2605313

feat: Add dynamic version flag, handle output collisions, add CONTRIB…

a8e16f3

…UTING.md

Copilot AI review requested due to automatic review settings February 20, 2026 15:38

Copilot started reviewing on behalf of ambicuity February 20, 2026 15:38 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

ambicuity added 2 commits February 20, 2026 09:57

test: Address inline code review feedback from Copilot

6d9e130

ambicuity changed the title ~~feat: Add dynamic version flag, handle output collisions, add CONTRIBUTING.md~~ feat: add type hints, logging, pre-commit, exit code docs, and Python API docs Feb 21, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add type hints, logging, pre-commit, exit code docs, and Python API docs#48

feat: add type hints, logging, pre-commit, exit code docs, and Python API docs#48
ambicuity wants to merge 5 commits intoSUPAIDEAS:mainfrom
ambicuity:fix/cli-enhancements-and-docs

ambicuity commented Feb 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Copilot AI Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if output_path.exists() and not getattr(args, 'force', False):
	if output_path.exists() and not args.force:

Conversation

ambicuity commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ambicuity commented Feb 20, 2026 •

edited

Loading