fix: bump to v0.3.0 with stricter tag matching #16
Conversation
TODO Report
Found 69 TODO(s) in 4 file(s): Summary by tag
Details by file
|
|
Warning Rate limit exceeded@alexandretrotel has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 16 minutes and 19 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughVersion 0.3.0 introduces stricter defaults (case_sensitive to true, require_colon to true) and adds new CLI flags (--ignore-case, --no-require-colon) for flexible matching. Refactors config handling via new CliOptions struct, updates TodoParser with with_options constructor, and deprecates with_regex. Removes zed extension from workspace. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cli/src/lib.rs (1)
327-327: Inconsistent case sensitivity default in cmd_stats.The
cmd_statsfunction usesTodoParser::new(&tags, false)with a hardcodedcase_sensitive: false, which is inconsistent with the new v0.3.0 defaults where case sensitivity should betrueby default. This means the stats command will use the old, more permissive matching behavior while scan/list use the strict defaults.🔧 Suggested fix to align with new defaults
// Get tags from CLI or config let tags = args.tags.clone().unwrap_or(config.tags.clone()); // Create parser and scanner - let parser = TodoParser::new(&tags, false); + let parser = TodoParser::with_options( + &tags, + config.case_sensitive, + config.require_colon, + config.custom_pattern.as_deref(), + ); let scanner = Scanner::new(parser, ScanOptions::default());
🤖 Fix all issues with AI agents
In @Cargo.toml:
- Line 3: Remove stale "extensions/zed" references from flake.nix: locate the
replaceStrings invocation that contains the "extensions/zed" replacement pattern
and delete that entry (or simplify the replaceStrings call if it becomes a
no-op), and update the filteredSrc definition to remove the exclusion of
"extensions/zed" (drop that path from the filter list). Ensure you only remove
the hardcoded "extensions/zed" patterns so replaceStrings and filteredSrc remain
valid for other entries.
In @cli/src/parser.rs:
- Around line 833-843: The test name and comments incorrectly reference Go while
using `::` syntax; rename the test `test_no_match_go_scope_resolution` to
something like `test_no_match_cpp_rust_scope_resolution` and update the inline
comments to say "C++/Rust scope resolution" or similar; keep the assertions and
the `TodoParser::new(...);` and `parse_line(...)` calls unchanged so the test
still verifies that `::`-style tokens (e.g., in `BindAddress { source:
std::io::Error },` and `fmt::Println("error")`) do not match.
🧹 Nitpick comments (2)
cli/src/cli.rs (2)
104-114: Clarify the comment about the default value.The comment says "default: true" but the field itself defaults to
false(line 137). The comment is describing the effective behavior when neither--case-sensitivenor--ignore-caseis passed, not the field's default value. This could confuse readers.📝 Suggested clarification
- /// Case-sensitive tag matching (default: true, only uppercase tags match) + /// Explicitly enable case-sensitive tag matching (overrides config) #[arg(long)] pub case_sensitive: bool, /// Ignore case when matching tags (matches TODO, todo, Todo, etc.) #[arg(long)] pub ignore_case: bool, /// Don't require colon after tag (allow "TODO something" without colon) #[arg(long)] pub no_require_colon: bool,
173-183: Same comment clarification needed here.Similar to the comment on
ScanArgs, the "default: true" comment could be clarified to avoid confusion about the field's default value vs. the effective behavior.📝 Suggested clarification
- /// Case-sensitive tag matching (default: true, only uppercase tags match) + /// Explicitly enable case-sensitive tag matching (overrides config) #[arg(long)] pub case_sensitive: bool, /// Ignore case when matching tags (matches TODO, todo, Todo, etc.) #[arg(long)] pub ignore_case: bool, /// Don't require colon after tag (allow "TODO something" without colon) #[arg(long)] pub no_require_colon: bool,
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
CHANGELOG.mdCargo.tomlREADME.mdcli/.todorc.example.jsoncli/.todorc.jsoncli/Cargo.tomlcli/src/cli.rscli/src/config.rscli/src/lib.rscli/src/parser.rscore/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (2)
cli/src/config.rs (1)
cli/src/cli.rs (1)
default(126-143)
cli/src/lib.rs (1)
cli/src/parser.rs (1)
with_options(72-86)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: Test (macos-latest, stable)
- GitHub Check: Test (windows-latest, stable)
- GitHub Check: Analyze (rust)
🔇 Additional comments (25)
cli/.todorc.json (1)
18-19: Breaking change is well-justified and documented.The stricter defaults (case_sensitive: true, require_colon: true) significantly reduce false positives in real-world code, such as namespace operators (
std::io::Error) and variable names (ERROR_CODE). The CHANGELOG provides a clear migration path for users who need the old behavior.CHANGELOG.md (1)
1-149: Excellent documentation of breaking changes.The CHANGELOG is comprehensive and user-friendly:
- Breaking changes are clearly marked and justified
- Migration guide provides multiple options with concrete examples
- Test coverage is documented, building confidence in the changes
- Follows Keep a Changelog format and semantic versioning
This is a model example of how to communicate breaking changes to users.
cli/.todorc.example.json (1)
8-9: Good practice: keeping example in sync with defaults.The example configuration correctly reflects the new stricter defaults, ensuring users who reference this file will understand the current behavior.
core/Cargo.toml (1)
3-3: Version bump correctly reflects breaking changes.The 0.2.1 → 0.3.0 increment follows semantic versioning for the breaking changes introduced (stricter default matching behavior). This aligns with the broader 0.3.0 release across the workspace.
cli/Cargo.toml (1)
3-3: LGTM! Version bump is consistent.The version bump to 0.3.0 for both the CLI package and its dependency on todo-tree-core is consistent with the PR objectives.
Also applies to: 28-28
README.md (2)
133-135: LGTM! Default values clearly documented.The JSON configuration example now shows the new strict defaults (
case_sensitive: trueandrequire_colon: true), which aligns with the version 0.3.0 changes.
184-235: Excellent documentation of the stricter defaults and flexibility options.The new sections clearly explain:
- The stricter tag format requirements (UPPERCASE + colon)
- Compliant vs. non-compliant examples
- CLI flags and config options for flexible matching
- Rationale for the strict defaults with concrete examples of false positives avoided
This will help users understand the breaking change and how to opt into the legacy behavior if needed.
cli/src/cli.rs (2)
138-140: LGTM! New fields initialized correctly.The default values for
ignore_caseandno_require_colonare correctly set tofalse, allowing the config defaults to take precedence when these flags aren't explicitly set.
279-280: LGTM! New fields propagated correctly.The conversion from
ScanArgstoListArgscorrectly propagates the newignore_caseandno_require_colonfields.cli/src/config.rs (3)
11-24: LGTM! Clean design for CLI options.The
CliOptionsstruct effectively encapsulates all CLI overrides and distinguishes between explicit flags (e.g.,case_sensitive: Option<bool>) and boolean flags (e.g.,ignore_case: bool), allowing proper precedence handling.
68-69: LGTM! Stricter defaults implemented correctly.The new defaults (
case_sensitive: true,require_colon: true) align with the PR objectives to reduce false positives.
143-187: LGTM! Merge logic handles precedence correctly.The merge logic properly handles the precedence:
- Explicit
--case-sensitiveflag takes precedence--ignore-caseflag forces case-insensitive matching--no-require-colondisables colon requirement- Config file values are used as fallback
This provides a clean way to override config settings via CLI.
cli/src/lib.rs (4)
49-88: LGTM! Options resolution logic is well-structured.The
cmd_scanfunction correctly:
- Builds
CliOptionsfrom CLI arguments- Merges them with config
- Resolves
case_sensitiveandrequire_colonwith proper precedence- Passes resolved options to
TodoParser::with_options
143-182: LGTM! cmd_list follows the same pattern.The
cmd_listfunction applies the same clean pattern ascmd_scanfor handling the new options.
720-725: LGTM! Tests updated consistently with new fields.All test cases for
ScanArgsare correctly updated to include the newignore_caseandno_require_colonfields with appropriate default values.Also applies to: 751-755, 782-786
872-875: LGTM! ListArgs tests also updated consistently.All test cases for
ListArgsare correctly updated to include the new fields.Also applies to: 899-901, 925-927
cli/src/parser.rs (9)
42-47: LGTM - Clear rationale for removing :: from default pattern.The documentation clearly explains why
::was removed to prevent false positives with scope resolution operators. The updated DEFAULT_REGEX is correct.
68-68: LGTM - Stricter default behavior aligns with v0.3.0 objectives.The
new()method now enforcesrequire_colon=trueby default, reducing false positives as intended.
71-86: LGTM - Well-designed constructor with clear parameter semantics.The
with_optionsconstructor provides fine-grained control over matching behavior while maintaining a clean API.
88-97: LGTM - Proper deprecation path for backward compatibility.The deprecated
with_regexmethod correctly delegates towith_optionswithrequire_colon=true, maintaining backward compatibility.
117-122: LGTM - Correct pattern modification for optional colon support.The logic correctly replaces
:(.*)with[:\\s]+(.*)whenrequire_colon=false, allowing either colons or whitespace as separators. The check forcustom_regex.is_none()ensures user-provided patterns are not modified.
314-333: LGTM - Thorough test coverage for colon requirement behavior.The test correctly verifies both the new default (require_colon=true) and the configurable option (require_colon=false).
700-747: LGTM - Test updated to reflect stricter defaults.The test appropriately verifies that markdown headings without colons no longer match with the new defaults, while still matching with the old-style configuration.
861-955: LGTM - Excellent test coverage for new default behaviors.These tests comprehensively verify:
- Default colon requirement (require_colon=true)
- Configurable colon optionality (require_colon=false)
- Default case sensitivity (case_sensitive=true)
- Configurable case insensitivity (case_sensitive=false)
The test assertions are clear and the coverage is thorough.
958-1067: LGTM - Comprehensive edge case coverage.These tests verify that the parser correctly handles various edge cases including Rust
Resulttypes, variable names containing tags, and different comment styles. The test coverage ensures the stricter defaults don't introduce false positives.
There was a problem hiding this comment.
Pull request overview
This PR bumps the version to 0.3.0 and introduces stricter tag matching defaults to reduce false positives in code scanning. The changes make tag matching case-sensitive and require a colon after tags by default, while providing CLI flags and configuration options for users who want the previous behavior.
Key changes:
- Default tag matching now requires uppercase tags with a colon (e.g.,
TODO:instead oftodoorTODO) - Removed
::from default comment markers to prevent false positives in Rust and C++ namespace code - Added
--ignore-caseand--no-require-colonCLI flags for flexible matching
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| core/Cargo.toml | Version bump to 0.3.0 |
| cli/Cargo.toml | Version bump to 0.3.0 and updated core dependency |
| cli/src/parser.rs | Updated regex pattern, added with_options() method, deprecated with_regex(), added comprehensive tests for new behavior |
| cli/src/lib.rs | Updated to use new parser options and handle new CLI flags |
| cli/src/config.rs | Added CliOptions struct, refactored merge_with_cli(), added require_colon field |
| cli/src/cli.rs | Added --ignore-case and --no-require-colon flags to scan and list commands |
| cli/.todorc.json | Updated defaults to case_sensitive: true and require_colon: true |
| cli/.todorc.example.json | Updated example configuration with new defaults |
| README.md | Added documentation for new defaults and migration guide |
| Cargo.toml | Removed non-existent extensions/zed from workspace |
| Cargo.lock | Updated dependencies and removed unused packages |
| CHANGELOG.md | Added comprehensive changelog documenting breaking changes and migration path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.