Add --line-format support to collection list command#23
Add --line-format support to collection list command#23DanielCardonaRojas merged 4 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 36 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe changes add a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/cli/handlers.rs (1)
2163-2226:⚠️ Potential issue | 🟡 MinorA few rough edges in the template rendering.
{COUNT}leaks literally in the--bookmarkbranch (Lines 2165-2181). The doc/top-level help lists{COUNT}as supported, but it isn't replaced here. At minimum strip/replace it with""in this branch so the placeholder doesn't appear in output.Value-level placeholder bleed-through. Because placeholders are replaced sequentially on the same string, any earlier substituted value that happens to contain a later placeholder (e.g. a collection
nameequal to"{CREATED}", or a description ending with{count}) will be re-substituted on the next.replace(..)call. Consider a single-pass formatter (e.g. small regex, orstrfmt-style loop) to avoid cross-field interference.No escape expansion for
\t/\n. Users invoking from a plain shell with--line-format "{NAME}\t{COUNT}"will get the literal backslash-t, not a tab. It works in the TOML inextras/television-integration/codemark-collections.tomlonly because TOML interprets\tat parse time. Consider expanding common escapes in the template before substitution so the CLI UX matches the TV config.Duplicated rendering logic between the two branches. Lines 2165-2181 and 2208-2226 differ only by the
{COUNT}row; extracting a helper (e.g.fn render_collection_line(template, &c, count: Option<usize>) -> String) would DRY this and make it easier to keep the two code paths in sync for future placeholders.Nit: the local
let short_id = output::short_id(&c.id);shadows the importedshort_idfunction in-scope; fine here, but renaming tosidor similar avoids confusion.Sketch of a single helper
fn render_collection_line( template: &str, c: &Collection, count: Option<usize>, ) -> String { let sid = output::short_id(&c.id); let desc = c.description.as_deref().unwrap_or(""); let count_str = count.map(|n| n.to_string()).unwrap_or_default(); // Consider a single-pass replacer here to avoid value-bleed. template .replace("{ID}", sid).replace("{id}", sid) .replace("{NAME}", &c.name).replace("{name}", &c.name) .replace("{COUNT}", &count_str).replace("{count}", &count_str) .replace("{DESCRIPTION}", desc).replace("{description}", desc) .replace("{CREATED}", &c.created_at).replace("{created}", &c.created_at) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/handlers.rs` around lines 2163 - 2226, The bookmark branch missing {COUNT} and duplicated, bleed-prone template logic should be refactored: add a helper fn render_collection_line(template: &str, c: &Collection, count: Option<usize>) -> String and use it from both places (replace the inline template blocks around the loops); inside the helper perform escape expansion on the template (translate \t, \n, \\) then do a single-pass substitution (e.g. iterate over placeholders with a small regex or tokenizer rather than chained .replace) to avoid value-level bleed-through, ensure count is substituted as "" when None (fixes the --bookmark branch), and rename the local short_id binding to sid to avoid shadowing the output::short_id function.extras/television-integration/codemark-collections.toml (1)
7-24:⚠️ Potential issue | 🟡 MinorRelative
--db .codemark/codemark.dbpaths will fail when launched from subdirectories.The three commands on lines 7, 12, and 24 explicitly pass relative paths to the database. When TV is launched with the current working directory in a subdirectory of the repo, these relative paths won't resolve and the commands will fail. The codemark CLI's
open_dbfunctions have git-root auto-detection viagit_context::detect_context(), but that only activates when--dbis omitted.Simplest fix: drop the explicit
--dbflag entirely to leverage the built-in auto-detection. Alternatively, derive an absolute path fromgit rev-parse --show-toplevel.Proposed diff
[source] -command = "codemark collection list --line-format \"{NAME}\t{COUNT}\t{DESCRIPTION}\" --db .codemark/codemark.db" +command = "codemark collection list --line-format \"{NAME}\t{COUNT}\t{DESCRIPTION}\"" display = "{split:\t:0} ({split:\t:1} bookmarks) {split:\t:2}" output = "{split:\t:0}" [preview] -command = "codemark collection show {split:\t:0} --format table --db .codemark/codemark.db" +command = "codemark collection show {split:\t:0} --format table" offset = "0" ... [actions.show] description = "Show bookmarks in this collection (opens television with codemark channel)" -command = "sh -c 'tv --source-command=\"codemark list --format tv --collection $1 --db .codemark/codemark.db\" codemark' placeholder {split:\t:0}" +command = "sh -c 'tv --source-command=\"codemark list --format tv --collection $1\" codemark' placeholder {split:\t:0}" mode = "execute"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extras/television-integration/codemark-collections.toml` around lines 7 - 24, The commands in the TOML (the root "command" value, "[preview] command", and "[actions.show] command") pass a relative "--db .codemark/codemark.db" which will break when launched from subdirectories; remove the explicit "--db .codemark/codemark.db" flags so the codemark CLI's built-in git-root auto-detection (git_context::detect_context) can find the DB, or if you prefer an absolute path, replace the flag with a computed absolute path via `git rev-parse --show-toplevel` before invoking the CLI; update the three command strings (command, [preview].command, and [actions.show].command) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@extras/television-integration/codemark-collections.toml`:
- Around line 7-24: The commands in the TOML (the root "command" value,
"[preview] command", and "[actions.show] command") pass a relative "--db
.codemark/codemark.db" which will break when launched from subdirectories;
remove the explicit "--db .codemark/codemark.db" flags so the codemark CLI's
built-in git-root auto-detection (git_context::detect_context) can find the DB,
or if you prefer an absolute path, replace the flag with a computed absolute
path via `git rev-parse --show-toplevel` before invoking the CLI; update the
three command strings (command, [preview].command, and [actions.show].command)
accordingly.
In `@src/cli/handlers.rs`:
- Around line 2163-2226: The bookmark branch missing {COUNT} and duplicated,
bleed-prone template logic should be refactored: add a helper fn
render_collection_line(template: &str, c: &Collection, count: Option<usize>) ->
String and use it from both places (replace the inline template blocks around
the loops); inside the helper perform escape expansion on the template
(translate \t, \n, \\) then do a single-pass substitution (e.g. iterate over
placeholders with a small regex or tokenizer rather than chained .replace) to
avoid value-level bleed-through, ensure count is substituted as "" when None
(fixes the --bookmark branch), and rename the local short_id binding to sid to
avoid shadowing the output::short_id function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fc2d30b0-1e92-413d-8691-52c166a25f19
📒 Files selected for processing (3)
extras/television-integration/codemark-collections.tomlsrc/cli/handlers.rssrc/cli/mod.rs
| /// Custom line format template (placeholders: {ID}, {NAME}, {COUNT}, {DESCRIPTION}, {CREATED}) | ||
| #[arg(long)] | ||
| pub line_format: Option<String>, |
There was a problem hiding this comment.
Doc lists {COUNT} but it's only populated when --bookmark is not used.
handle_collection_list only substitutes {COUNT}/{count} in the "list all collections" branch; the --bookmark <id> branch (which iterates list_collections_for_bookmark) has no count and leaves {COUNT} as a literal in the output. Either document the restriction or substitute it with a sensible value (e.g. empty string or per-collection count) in the bookmark branch.
Suggested doc clarification
- /// Custom line format template (placeholders: {ID}, {NAME}, {COUNT}, {DESCRIPTION}, {CREATED})
+ /// Custom line format template (placeholders: {ID}, {NAME}, {DESCRIPTION}, {CREATED};
+ /// additionally {COUNT} when --bookmark is not used)
#[arg(long)]
pub line_format: Option<String>,
1 participant
Summary
--line-formatoption tocodemark collection listfor custom output formatting{ID},{NAME},{COUNT},{DESCRIPTION},{CREATED}cdprefixChanges
src/cli/mod.rs: Addline_formatfield toCollectionListArgssrc/cli/handlers.rs: Implement custom line format rendering inhandle_collection_listextras/television-integration/codemark-collections.toml: Use--line-formatand simplify commandsTest plan
--line-formatworks with various placeholder combinationsSummary by CodeRabbit
New Features
--line-formatparameter to collection list command for custom output formatting. Users can define templates with placeholders for collection ID, name, count, description, and creation date.Chores