fix(tasks): honor category argument in add_task (#46)#63
Conversation
📝 WalkthroughWalkthroughThe PR adds category argument support to the ChangesTask Category Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
trushell/commands/tasks.py (1)
28-34: ⚡ Quick winEdge case: Empty quoted task is not rejected.
If a user provides an empty quoted task (e.g.,
task add ""), the regex won't match because[^"]+requires at least one character. The else branch on line 33 will treat the input""(including literal quote characters) as the task text, creating a Todo withtask='""'andcategory='General'. The validation on line 24 doesn't catch this because'""'.strip()evaluates to'""'(non-empty).Consider adding validation after parsing to reject empty task text:
🛡️ Proposed validation for empty task
else: task_text = text category = "General" + + if not task_text: + print('Usage: task add "<task>" ["<category>"]') + return todo = Todo(task=task_text, category=category)Note: The pattern
[^"]+also doesn't handle embedded quotes (e.g.,"Buy \"milk\""). This is a known limitation of the simple regex approach. For more robust parsing, considershlex.split()in a future enhancement, though that would require careful handling of backwards compatibility for unquoted input.🤖 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 `@trushell/commands/tasks.py` around lines 28 - 34, The parsing allows an empty quoted task like '""' to slip through as task_text, so after the existing parsing (the match / else block that sets match, task_text and category) add a validation that rejects empty quoted tasks: detect if the original text starts and ends with a quote (e.g., text.startswith('"') and text.endswith('"')) and then check the inner content (text[1:-1].strip()) is not empty; if it is empty, raise the same validation/return error used on line 24. Apply this check after the match/else logic (referencing variables match, task_text, category and the surrounding parsing block) so inputs like '""' are rejected while preserving existing behavior for unquoted inputs.
🤖 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.
Nitpick comments:
In `@trushell/commands/tasks.py`:
- Around line 28-34: The parsing allows an empty quoted task like '""' to slip
through as task_text, so after the existing parsing (the match / else block that
sets match, task_text and category) add a validation that rejects empty quoted
tasks: detect if the original text starts and ends with a quote (e.g.,
text.startswith('"') and text.endswith('"')) and then check the inner content
(text[1:-1].strip()) is not empty; if it is empty, raise the same
validation/return error used on line 24. Apply this check after the match/else
logic (referencing variables match, task_text, category and the surrounding
parsing block) so inputs like '""' are rejected while preserving existing
behavior for unquoted inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cf716379-26e4-4de6-b720-b834903c1c2e
📒 Files selected for processing (3)
tests/test_tasks.pytrushell/cli.pytrushell/commands/tasks.py
Summary
Fixes #46.
add_task()hard-codedcategory="General"and treated the entire argument (quotes included) as the task text, so a user-supplied category was silently discarded — even though theTodomodel and the SQLite persistence layer fully support categories and the README documentsaddtask "<task>" "<category>".Fix
add_tasknow parses an optional quoted category, mirroring the existingupdate_tasksyntax and the documented form. Fully backward-compatible:task add "Buy milk" "Shopping"Buy milkShoppingtask add "Pay rent"Pay rentGeneral(default)task add buy groceriesbuy groceriesGeneral(unchanged)A one-line companion change in
cli.pymakes itsaddtaskhandler pass the quoted form instead of concatenating the two captured groups into a single string.Tests
Added 3 regression tests in
tests/test_tasks.pycovering a supplied category, the default fallback, and unquoted back-compat. Full suite passes locally — 31 passed, 5 skipped on Python 3.10.Summary by CodeRabbit
New Features
Tests