Skip to content

Commit 4fa7458

Browse files
author
kiran-garre
committed
fix: Fix several todo list related bugs
Updates: - Added a Lookup command for the todo_list tool that allows the model to see existing todo lists in the user's current directory - Added ID validation so that the model is notified that a given ID is invalid, rather than throwing an error - Changed todo list checkboxes to be "[ ]" and "[x]" for cross-platform consistency - Changed todo list selection to use the same theme as everything else
1 parent 0b80693 commit 4fa7458

File tree

3 files changed

+62
-13
lines changed

3 files changed

+62
-13
lines changed

crates/chat-cli/src/cli/chat/cli/todos.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crossterm::style::{
44
self,
55
Stylize,
66
};
7-
use dialoguer::FuzzySelect;
7+
use dialoguer::Select;
88
use eyre::Result;
99

1010
use crate::cli::chat::tools::todo::{
@@ -196,7 +196,7 @@ impl TodoSubcommand {
196196
}
197197

198198
fn fuzzy_select_todos(entries: &[TodoDisplayEntry], prompt_str: &str) -> Option<usize> {
199-
FuzzySelect::new()
199+
Select::with_theme(&crate::util::dialoguer_theme())
200200
.with_prompt(prompt_str)
201201
.items(entries)
202202
.report(false)

crates/chat-cli/src/cli/chat/tools/todo.rs

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ fn queue_next_without_newline(output: &mut impl Write, task: String, completed:
9292
if completed {
9393
queue!(
9494
output,
95-
style::SetAttribute(style::Attribute::Italic),
9695
style::SetForegroundColor(style::Color::Green),
97-
style::Print(" ■ "),
96+
style::Print("[x] "),
97+
style::SetAttribute(style::Attribute::Italic),
9898
style::SetForegroundColor(style::Color::DarkGrey),
9999
style::Print(task),
100100
style::SetAttribute(style::Attribute::NoItalic),
@@ -103,7 +103,7 @@ fn queue_next_without_newline(output: &mut impl Write, task: String, completed:
103103
queue!(
104104
output,
105105
style::SetForegroundColor(style::Color::Reset),
106-
style::Print(format!(" {task}")),
106+
style::Print(format!("[ ] {task}")),
107107
)?;
108108
}
109109
Ok(())
@@ -207,10 +207,22 @@ pub enum TodoList {
207207
new_description: Option<String>,
208208
current_id: String,
209209
},
210+
211+
// Shows the model the IDs of all existing todo lists
212+
Lookup,
210213
}
211214

212215
impl TodoList {
213216
pub async fn invoke(&self, os: &Os, output: &mut impl Write) -> Result<InvokeOutput> {
217+
if let Some(id) = self.get_id() {
218+
if !os.fs.exists(id_to_path(os, &id)?) {
219+
let error_string = "No todo list exists with the given ID";
220+
queue!(output, style::Print(error_string.yellow()))?;
221+
return Ok(InvokeOutput {
222+
output: super::OutputKind::Text(error_string.to_string()),
223+
});
224+
}
225+
}
214226
let (state, id) = match self {
215227
TodoList::Create {
216228
tasks,
@@ -321,6 +333,31 @@ impl TodoList {
321333
state.display_list(output)?;
322334
(state, id.clone())
323335
},
336+
TodoList::Lookup => {
337+
let path = get_todo_list_dir(os)?;
338+
queue!(output, style::Print("Finding existing todo lists...".yellow()))?;
339+
if os.fs.exists(&path) {
340+
let mut ids = Vec::new();
341+
let mut entries = os.fs.read_dir(&path).await?;
342+
while let Some(entry) = entries.next_entry().await? {
343+
let entry_path = entry.path();
344+
ids.push(
345+
entry_path
346+
.file_stem()
347+
.map(|f| f.to_string_lossy().to_string())
348+
.unwrap_or_default(),
349+
);
350+
}
351+
if !ids.is_empty() {
352+
return Ok(InvokeOutput {
353+
output: super::OutputKind::Text(format!("Existing todo IDs:\n {}", ids.join("\n"))),
354+
});
355+
}
356+
}
357+
return Ok(InvokeOutput {
358+
output: super::OutputKind::Text("No todo lists in the user's current directory.".to_string()),
359+
});
360+
},
324361
};
325362

326363
let invoke_output = format!("TODO LIST STATE: {}\n\n ID: {id}", serde_json::to_string(&state)?);
@@ -330,6 +367,12 @@ impl TodoList {
330367
}
331368

332369
pub async fn validate(&mut self, os: &Os) -> Result<()> {
370+
// Rather than throwing an error, let invoke() handle this case
371+
if let Some(id) = self.get_id() {
372+
if !os.fs.exists(id_to_path(os, &id)?) {
373+
return Ok(());
374+
}
375+
}
333376
match self {
334377
TodoList::Create {
335378
tasks,
@@ -361,12 +404,6 @@ impl TodoList {
361404
}
362405
}
363406
},
364-
TodoList::Load { load_id: id } => {
365-
let state = TodoListState::load(os, id).await?;
366-
if state.tasks.is_empty() {
367-
bail!("Loaded todo list is empty");
368-
}
369-
},
370407
TodoList::Add {
371408
new_tasks,
372409
insert_indices,
@@ -406,9 +443,20 @@ impl TodoList {
406443
}
407444
}
408445
},
446+
TodoList::Load { .. } | TodoList::Lookup => (),
409447
}
410448
Ok(())
411449
}
450+
451+
pub fn get_id(&self) -> Option<String> {
452+
match self {
453+
TodoList::Add { current_id, .. }
454+
| TodoList::Complete { current_id, .. }
455+
| TodoList::Remove { current_id, .. } => Some(current_id.clone()),
456+
TodoList::Load { load_id } => Some(load_id.clone()),
457+
TodoList::Create { .. } | TodoList::Lookup => None,
458+
}
459+
}
412460
}
413461

414462
/// Generated by Q

crates/chat-cli/src/cli/chat/tools/tool_index.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,10 @@
309309
"complete",
310310
"load",
311311
"add",
312-
"remove"
312+
"remove",
313+
"lookup"
313314
],
314-
"description": "The command to run. Allowed options are `create`, `complete`, `load`, `add`, and `remove`."
315+
"description": "The command to run. Allowed options are `create`, `complete`, `load`, `add`, `remove`, and `lookup`. Call `lookup` without arguments to see a list of all existing TODO list IDs."
315316
},
316317
"tasks": {
317318
"description": "Required parameter of `create` command containing the list of DISTINCT tasks to be added to the TODO list.",

0 commit comments

Comments
 (0)