-
Notifications
You must be signed in to change notification settings - Fork 2
Add 'Ignore Tags' feature to TODO/DONE triggers #12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Updated README to clarify configuration options and added support for 'Ignore Tags' to skip TODO/DONE triggers based on specified tags. - Implemented logic in the extension to handle ignored tags during TODO/DONE transitions.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds an "Ignore Tags" extension setting that accepts a comma- or pipe-separated list of tags to skip TODO/DONE triggers. Implements a new helper Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 18: Fix the duplicated word "the the" in the README descriptions for the
`/Today` entry (and the duplicate occurrence later) by replacing "inserts the
the current day" with "inserts the current day" wherever the `/Today`
description appears; search for the literal string "/Today" to locate both
occurrences and update the text accordingly.
🧹 Nitpick comments (4)
src/index.ts (4)
433-442: Consider merging the twoif (block)checks.The double
if (block)at Lines 433 and 440 works correctly but reads awkwardly. These can be consolidated into a single conditional.♻️ Suggested consolidation
- if (block) { - const blockUid = getBlockUidFromTarget(input); - if (blockUid && shouldIgnoreBlock(getTextByBlockUid(blockUid))) { - unstyleBlock(block); - return; - } - } - if (block) { - styleBlock(block); + if (block) { + const blockUid = getBlockUidFromTarget(input); + if (blockUid && shouldIgnoreBlock(getTextByBlockUid(blockUid))) { + unstyleBlock(block); + } else { + styleBlock(block); + } }
454-463: Redundant ignore guard — unchecked blocks always get unstyled.In the unchecked (
else) branch, both the ignore path (Line 457) and the normal path (Line 462) callunstyleBlock(block). TheshouldIgnoreBlockcheck and early return at Lines 454-460 have no observable effect and can be removed to simplify the code.♻️ Suggested simplification
const block = CLASSNAMES_TO_CHECK.map( (c) => l.closest(`.${c}`) as HTMLElement, ).find((d) => !!d); - if (block) { - const blockUid = getBlockUidFromTarget(input); - if (blockUid && shouldIgnoreBlock(getTextByBlockUid(blockUid))) { - unstyleBlock(block); - return; - } - } if (block) { unstyleBlock(block); }
301-327: Click handler on checkbox doesn't guard againstonDonereturning early for ignored blocks.When
input.checkedisfalse(Line 315),onDoneis called and the return value's.explodeproperty is accessed at Line 317. This works correctly today becauseshouldIgnoreBlockreturns{ explode: false }fromonDone. However, there's a latent fragility: theonTodopath (Line 314) discards the return value, yet theonDonepath depends on it. If a future refactor changesonDone's early-return toundefinedorvoid, Line 317 would throw at runtime (Cannot read properties of undefined).Consider a defensive check:
const config = onDone(blockUid, oldValue); - if (config.explode) { + if (config?.explode) {This is not a current bug — just a minor resilience improvement.
403-404: Pre-existing:isStrikethroughandisClassnameare read once at init, not reactively.This isn't introduced by this PR, but worth noting: if a user toggles the Strikethrough or Classname settings after the extension loads, the change won't take effect until Roam is reloaded. The new
shouldIgnoreBlockfunction reads settings on each call (which is reactive), creating a behavioral inconsistency. Not actionable for this PR, but something to be aware of.
…ock UID and ignore tags. This change enhances the handling of zoom items by unstyling blocks that should be ignored.
…lder in configuration options for TODO/DONE triggers.
Summary by CodeRabbit
New Features
Documentation