Conversation
📝 WalkthroughWalkthroughAcross five modules, this PR removes precomputed/consolidated lookup tables ( ChangesAPI Cleanup and Lookup Simplification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the JobBinds addon by removing unused helper APIs and simplifying key-blocking/profile-loading logic, reducing redundant work and tightening the flow between profile execution and UI refresh.
Changes:
- Removed unused helper functions and reverse-lookup tables from
vk_codes.lua,ui_functions.lua, andkeyboard_ui.lua. - Simplified blocked-key checks by removing the consolidated
all_blockedtable and relying directly onM.blocked. - Streamlined profile loading/UI update paths in
jobbinds.luaand simplified macro directory creation inui_functions.lua.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vk_codes.lua | Removes unused reverse lookup + helper APIs, keeping only the mapping and key-name helpers used by the UI. |
| ui_functions.lua | Removes unused helpers, simplifies blocked-key validation to use the module’s returned message, and inlines mkdir with pcall. |
| keyboard_ui.lua | Removes unused UI control helpers and minor comment cleanup; keeps profile loading via load_bindings_from_profile. |
| jobbinds.lua | Simplifies UI update helper usage and avoids recomputing the profile path when reading tracked keys. |
| blocked_keybinds.lua | Removes all_blocked consolidation and simplifies checks to directly reference M.blocked. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@ui_functions.lua`:
- Around line 274-277: The create_macro_file function has a race condition where
the `/mkdir` command is queued asynchronously with QueueCommand but the
subsequent file write via io.open happens immediately in the same execution
thread before the directory is created. To fix this, either implement polling
logic to wait for the directory to exist before calling io.open on the file
path, or refactor the function to defer the actual file write operation until
after the directory creation completes (potentially by using a callback or
moving the io.open call to be executed after confirming the scripts_path
directory exists).
🪄 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
Run ID: d9cc2332-3dce-4010-b829-0e55e7d306d4
📒 Files selected for processing (5)
blocked_keybinds.luajobbinds.luakeyboard_ui.luaui_functions.luavk_codes.lua
💤 Files with no reviewable changes (2)
- vk_codes.lua
- keyboard_ui.lua
| function ui_functions.create_macro_file(macro_name, content, existing_command) | ||
| local scripts_path = ui_functions.get_scripts_path() | ||
| ensure_directory_exists(scripts_path) | ||
| pcall(function() AshitaCore:GetChatManager():QueueCommand(1, string.format('/mkdir "%s"', scripts_path)) end) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Ashita Lua API, does AshitaCore:GetChatManager():QueueCommand(delay, command)execute asynchronously relative to subsequent Lua statements? If/mkdir ""is queued andio.open("/file.txt", "w")is called immediately after, canio.open run before directory creation?
💡 Result:
In Ashita Lua (ADK) API, QueueCommand is not documented as “asynchronous relative to the next Lua statement.” Instead, it queues a chat/console command to be executed by Ashita’s command system at some later tick based on the provided delay argument. Example addon code shows QueueCommand being called on a later event (“timerpulse”/“render”) and that the delay parameter is used to control when the queued command is sent. Because the command is queued (not executed inline), if you queue a game command like /mkdir "" and then immediately call io.open("/file.txt","w") in the same Lua execution thread, directory creation performed by the queued in-game command may not have happened yet. That means io.open can run before the directory exists. In other words: - QueueCommand(delay, command) schedules the command; it should not be assumed to have completed before subsequent Lua lines run. - With a non-negative delay (e.g., 1), it will run on a subsequent update/tick rather than immediately, so a race is possible if you rely on filesystem effects from the queued command. Practically, to avoid the race you should (a) queue the /mkdir with a delay of -1 only if you know it is processed immediately by Ashita, or (b) wait/yield/poll for directory existence before calling io.open (e.g., schedule io.open in a later timer callback after the command has had time to execute). Evidence: Ashita addon documentation lists GetChatManager and IChatManager methods including QueueCommand/ParseCommand, and community code uses QueueCommand with explicit delay values in event callbacks rather than expecting immediate completion. For example, QueueCommand is called inside an on-render timer loop with delay=1, demonstrating that delay controls when the command is processed rather than executing within the same Lua statement flow [1]. Additional usage examples show QueueCommand being invoked during OnLoad/per-event setup with delay values (e.g., -1) without any mechanism that would make it synchronous with filesystem operations [2].
Citations:
- 1: https://git.ashitaxi.com/Addons/repeater/raw/commit/46a381ee146d77d2e4459f3edc323aa7fef86d1e/repeater.lua
- 2: https://thornyffxi.github.io/LuAshitacast/tutorial.html
Fix the /mkdir race condition before file creation.
At Line 276, /mkdir is queued with delay 1 via QueueCommand, but io.open(...) is called immediately after in the same Lua execution thread. QueueCommand schedules the command for a later game tick and does not execute synchronously—the directory may not exist when io.open runs, causing macro file creation to fail on first use. Either poll/yield for directory existence before calling io.open, or defer the file write to a callback after directory creation completes.
🤖 Prompt for 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.
In `@ui_functions.lua` around lines 274 - 277, The create_macro_file function has
a race condition where the `/mkdir` command is queued asynchronously with
QueueCommand but the subsequent file write via io.open happens immediately in
the same execution thread before the directory is created. To fix this, either
implement polling logic to wait for the directory to exist before calling
io.open on the file path, or refactor the function to defer the actual file
write operation until after the directory creation completes (potentially by
using a callback or moving the io.open call to be executed after confirming the
scripts_path directory exists).
Summary by CodeRabbit