Skip to content

Fix slash command native removal#4

Merged
mdroidian merged 2 commits intoRoamJS:mainfrom
mdroidian:fix-slash-command-native-removal
Feb 7, 2026
Merged

Fix slash command native removal#4
mdroidian merged 2 commits intoRoamJS:mainfrom
mdroidian:fix-slash-command-native-removal

Conversation

@mdroidian
Copy link
Copy Markdown
Contributor

@mdroidian mdroidian commented Feb 7, 2026


Open with Devin

Summary by CodeRabbit

  • Bug Fixes

    • Added error handling for slash command registration to catch and log errors properly
  • Refactor

    • Enhanced slash command invocation with improved context-aware target block resolution and insertion point calculation
    • Streamlined callback handling and removed legacy processing logic

…g the callback logic into a separate function. This improves code readability and maintains functionality for GIF insertion and context handling.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 7, 2026

Walkthrough

The pull request updates the SlashCommandApi in src/index.ts by modifying the addCommand callback signature to return string | null | void instead of void. A new slashCallback function was introduced to handle slash command invocations, which resolves the target block UID from context or active textarea and computes an insertAt position before opening the Giphy picker. The previous inline callback logic has been replaced with this new pattern. Error handling via try/catch was added around slashCommand.addCommand registration, and legacy helper functions were removed as part of the refactor.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix slash command native removal' directly relates to the core change: refactoring the slash command callback mechanism to properly handle native removal and block UID resolution.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/index.ts (1)

35-36: window.roamAlphaAPI accessed without null safety.

Line 35 accesses window.roamAlphaAPI.ui.getFocusedBlock() directly, while slashCommand on lines 58–62 is accessed with optional chaining. If the slash callback ever fires in a context where roamAlphaAPI is unavailable, this will throw. Consider guarding with optional chaining for consistency:

Suggested diff
-    const focusedUid = window.roamAlphaAPI.ui.getFocusedBlock()?.["block-uid"] || "";
+    const focusedUid = window.roamAlphaAPI?.ui?.getFocusedBlock()?.["block-uid"] || "";

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.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@mdroidian mdroidian merged commit b327368 into RoamJS:main Feb 7, 2026
1 of 2 checks passed
@mdroidian mdroidian deleted the fix-slash-command-native-removal branch February 7, 2026 21:00
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