chore: add explicit UTF-8 encoding for FFI doc generation #322

xdustinface · 2025-12-28T10:58:24Z

Add encoding='utf-8' to all file open operations in the FFI documentation generation scripts since the default Windows encoding doesnt seem to go well with it and lets the relevant pre-commit step fail for Windows in #253.

Also improved error visibility in verify_ffi.py by capturing stderr when the subprocess fails.

Summary by CodeRabbit

Bug Fixes
- Improved error reporting to capture and display failure messages during documentation generation processes.
- Ensured proper UTF-8 text encoding to guarantee consistent character handling across all platforms.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Add `encoding='utf-8'` to all file open operations in the FFI documentation generation scripts since the default Windows encoding doesnt seem to go well with it and lets the relevant `pre-commit` step fail for Windows in #253. Also improved error visibility in `verify_ffi.py` by capturing stderr when the subprocess fails.

coderabbitai · 2025-12-28T10:58:37Z

📝 Walkthrough

Walkthrough

Three Python scripts updated: one to capture stderr instead of stdout on subprocess failures for improved error reporting, and two to explicitly specify UTF-8 encoding for file I/O operations in documentation generation scripts.

Changes

Cohort / File(s)	Summary
Error Handling `contrib/verify_ffi.py`	Modified `generate_ffi_docs()` to return stderr from subprocess when exit code is non-zero; previously always returned stdout regardless of failure status
UTF-8 Encoding Specification `dash-spv-ffi/scripts/generate_ffi_docs.py`, `key-wallet-ffi/scripts/generate_ffi_docs.py`	Added explicit `encoding='utf-8'` parameter to file open operations in `extract_ffi_functions()` (reading) and `main()` (writing) to ensure consistent Unicode handling across platforms

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 Stderr now whispers what stdout won't tell,
UTF-8 encoded in every shell,
No more encoding guesses in the dark,
Error messages shine—a helpful spark! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding explicit UTF-8 encoding for FFI documentation generation scripts, which aligns with the primary objectives of the PR.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch chore/ffi-utf8

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3884314 and fdfe8ce.

📒 Files selected for processing (3)

contrib/verify_ffi.py
dash-spv-ffi/scripts/generate_ffi_docs.py
key-wallet-ffi/scripts/generate_ffi_docs.py

🧰 Additional context used

🧠 Learnings (7)

📓 Common learnings

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)