Skip to content

Fix Tao port review findings - #8

Merged
OlympusLedgerOrg merged 5 commits into
devfrom
split-tao-review-fixes-2026-07-09
Jul 10, 2026
Merged

Fix Tao port review findings#8
OlympusLedgerOrg merged 5 commits into
devfrom
split-tao-review-fixes-2026-07-09

Conversation

@OlympusLedgerOrg

@OlympusLedgerOrg OlympusLedgerOrg commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Summary

  • applies the real Tao findings from the local CodeRabbit review
  • covers platform safety fixes, example cleanup, SPDX/docs nits, and macro parser validation
  • leaves root and Wry changes to separate PRs

Split

Part 2 of 3 replacing #6.

Validation

  • cargo fmt --manifest-path ports/tao/Cargo.toml --all --check
  • cargo check --manifest-path ports/tao/Cargo.toml --all-targets
  • git diff --check origin/dev..HEAD

Summary by CodeRabbit

  • Bug Fixes
    • Remapped Linux/JIS keys and corrected several keyboard scancode mappings; improved input handling and prevented crashes from invalid sizes, missing surfaces, failed display connections, and malformed URLs.
    • Improved cross-platform window, cursor-grab, fullscreen, drag-and-drop, taskbar progress, touch handling, and high-contrast behavior, with safer shutdown handling in examples.
  • Documentation
    • Updated README headings/badges, fixed minor wording/typos, and refreshed license metadata.
  • Examples
    • Hardened progress bar handling, cursor grab error reporting, custom events dispatch, multithreaded logic, redraw-thread shutdown, and transparency resize guarding.
  • Tests
    • Added an additional compile-time send conformance check.
  • Chores
    • Broadened CI lint/audit runs to cover all pull requests.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e35da5ad-9b3c-4f09-b602-912aaa6d552c

📥 Commits

Reviewing files that changed from the base of the PR and between 404f750 and cc70af0.

📒 Files selected for processing (2)
  • .github/workflows/audit.yml
  • .github/workflows/lint-rust.yml
💤 Files with no reviewable changes (2)
  • .github/workflows/lint-rust.yml
  • .github/workflows/audit.yml

📝 Walkthrough

Walkthrough

TAO receives platform-specific safety and behavior fixes across Android, iOS, Linux, and Windows. Examples improve error and shutdown handling, procedural macros reject trailing input, Send coverage expands, pull-request workflows broaden, and repository metadata and documentation are corrected.

Changes

TAO platform updates

Layer / File(s) Summary
Metadata and documentation updates
ports/tao/.changes/*, ports/tao/LICENSE.spdx, ports/tao/README.md, ports/tao/examples/README.md, ports/tao/src/icon.rs
Corrects changelog, SPDX, badge, heading, wording, and icon-validation documentation.
Example error and shutdown handling
ports/tao/examples/*.rs
Examples handle failed cursor and event operations, empty video-mode lists, ignored progress keys, redraw-thread shutdown, and zero-sized resize dimensions safely.
Android and iOS interop handling
ports/tao/src/platform_impl/android/ndk_glue.rs, ports/tao/src/platform_impl/ios/*
Android stores JNI context references as GlobalRef; iOS converts badge counts for FFI and skips invalid activity URLs instead of panicking.
Linux backend behavior and mappings
ports/tao/src/platform_impl/linux/*
Adds display and surface checks, defers device-loop termination until cleanup, corrects scancode mappings, resets taskbar caches, reports unsupported operations, and removes unsafe Window Send/Sync implementations.
Windows backend interop and state handling
ports/tao/src/platform_impl/windows/*
Uses initialized FFI buffers, corrects keyboard and locale mappings, hardens drag-and-drop and COM paths, preserves fullscreen state, and updates visibility and cursor clipping behavior.
Macro input validation and Send coverage
ports/tao/tao-macros/src/lib.rs, ports/tao/tests/send_objects.rs
Macro parsers reject unexpected trailing tokens, and EventLoopProxy<()> is added to Send conformance coverage.
Pull request workflow coverage
.github/workflows/audit.yml, .github/workflows/lint-rust.yml
Audit and Rust lint workflows no longer restrict pull-request runs by changed file paths.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 43.48% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the PR’s main purpose: addressing review findings in the Tao port.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch split-tao-review-fixes-2026-07-09

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ports/tao/examples/multithreaded.rs`:
- Around line 49-53: Prevent the Alt+F exclusive-fullscreen path from unwrapping
a missing video mode: update the relevant fullscreen toggle logic around the
video mode lookup to handle an empty `video_modes` list by skipping the
transition or returning `None`, and ensure this also covers the initial
empty-list state. Use the existing `video_mode_id` handling and
`video_modes.iter().nth(...)` lookup as anchors.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 80779afd-f65d-40d1-91c4-15698d8a8e9e

📥 Commits

Reviewing files that changed from the base of the PR and between 67719ec and 4b1d8d9.

📒 Files selected for processing (31)
  • ports/tao/.changes/fix-jis-key-mapping.md
  • ports/tao/.changes/readme.md
  • ports/tao/LICENSE.spdx
  • ports/tao/README.md
  • ports/tao/examples/README.md
  • ports/tao/examples/cursor_grab.rs
  • ports/tao/examples/custom_events.rs
  • ports/tao/examples/multithreaded.rs
  • ports/tao/examples/progress_bar.rs
  • ports/tao/examples/request_redraw_threaded.rs
  • ports/tao/examples/transparent.rs
  • ports/tao/src/icon.rs
  • ports/tao/src/platform_impl/android/ndk_glue.rs
  • ports/tao/src/platform_impl/ios/badge.rs
  • ports/tao/src/platform_impl/ios/scene.rs
  • ports/tao/src/platform_impl/linux/device.rs
  • ports/tao/src/platform_impl/linux/event_loop.rs
  • ports/tao/src/platform_impl/linux/keycode.rs
  • ports/tao/src/platform_impl/linux/taskbar.rs
  • ports/tao/src/platform_impl/linux/window.rs
  • ports/tao/src/platform_impl/windows/dark_mode.rs
  • ports/tao/src/platform_impl/windows/drop_handler.rs
  • ports/tao/src/platform_impl/windows/event_loop.rs
  • ports/tao/src/platform_impl/windows/keyboard.rs
  • ports/tao/src/platform_impl/windows/keyboard_layout.rs
  • ports/tao/src/platform_impl/windows/monitor.rs
  • ports/tao/src/platform_impl/windows/raw_input.rs
  • ports/tao/src/platform_impl/windows/window.rs
  • ports/tao/src/platform_impl/windows/window_state.rs
  • ports/tao/tao-macros/src/lib.rs
  • ports/tao/tests/send_objects.rs

Comment thread ports/tao/examples/multithreaded.rs
@OlympusLedgerOrg
OlympusLedgerOrg merged commit 2c32cd1 into dev Jul 10, 2026
16 checks passed
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.

1 participant