Skip to content

move history txt to state dir #5410

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.

Sign up for GitHub

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

Merged
zanesq merged 4 commits into from
Oct 28, 2025
Merged

move history txt to state dir #5410

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8d036d3
move history txt to state dir
zanesq Oct 28, 2025
1aeeb65
clippy
zanesq Oct 28, 2025
9359d76
remove duplication and overcomplication
zanesq Oct 28, 2025
a4c9f67
feedback
zanesq Oct 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions crates/goose-cli/src/session/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,24 +397,37 @@ impl CliSession {
let completer = GooseCompleter::new(self.completion_cache.clone());
editor.set_helper(Some(completer));

let history_file = Paths::config_dir().join("history.txt");
let history_file = Paths::state_dir().join("history.txt");
let old_history_file = Paths::config_dir().join("history.txt");

Copy link
Collaborator

@DOsinga DOsinga Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the above seems more complicated than it should be. we already check if the history file exists. if we also check whether the old history exists we can then just load from old if new doesn't exist and delete old when new does exist

if let Some(parent) = history_file.parent() {
if !parent.exists() {
std::fs::create_dir_all(parent)?;
}
}

if history_file.exists() {
let history_loaded = if history_file.exists() {
if let Err(err) = editor.load_history(&history_file) {
eprintln!("Warning: Failed to load command history: {}", err);
}
}
true
} else if old_history_file.exists() {
if let Err(err) = editor.load_history(&old_history_file) {
eprintln!("Warning: Failed to load command history: {}", err);
}
true
} else {
false
};
Copy link
Collaborator

@DOsinga DOsinga Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd probably go with something like:

let load_from = [&history_file, &old_history_file]
.iter()
.find(|f| f.exists());

if let Some(file) = load_from {
if let Err(err) = editor.load_history(file) {
eprintln!("Warning: Failed to load command history: {}", err);
}
}


let save_history =
|editor: &mut rustyline::Editor<GooseCompleter, rustyline::history::DefaultHistory>| {
if let Err(err) = editor.save_history(&history_file) {
eprintln!("Warning: Failed to save command history: {}", err);
} else if history_loaded && old_history_file.exists() {
Copy link
Collaborator

@DOsinga DOsinga Oct 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to check on history_loaded here I think

if let Err(err) = std::fs::remove_file(&old_history_file) {
eprintln!("Warning: Failed to remove old history file: {}", err);
}
}
};

Expand Down
Loading