Skip to content

Commit

Permalink
fix(tui): no longer error if preferences cannot be read (#10004)
Browse files Browse the repository at this point in the history 
### Description

Fixes #9999

The TUI should function even if we fail at loading preferences. 

### Testing Instructions

Add test to verify that if the preferences file contains invalid JSON a
loader is still created. User probably was hitting a perms issue or some
other FS issue and not an invalid JSON, but the test is just meant to
show we don't crash.
  • Loading branch information
@chris-olszewski
chris-olszewski authored Feb 19, 2025
1 parent cd8c515 commit 3e774d3
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 31 deletions.
44 changes: 22 additions & 22 deletions crates/turborepo-ui/src/tui/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl<W> App<W> {
fn update_task_selection_pinned_state(&mut self) -> Result<(), Error> {
// Preferences assume a pinned state when there is an active task.
// This `None` creates "un-pinned-ness" on the next TUI startup.
self.preferences.set_active_task(None)?;
self.preferences.set_active_task(None);
Ok(())
}

Expand All @@ -166,7 +166,7 @@ impl<W> App<W> {
self.preferences.set_active_task(
self.is_task_selection_pinned
.then(|| active_task.to_owned()),
)?;
);
Ok(())
}

Expand Down Expand Up @@ -607,7 +607,7 @@ pub async fn run_app(
) -> Result<(), Error> {
let mut terminal = startup(color_config)?;
let size = terminal.size()?;
let preferences = PreferenceLoader::new(repo_root)?;
let preferences = PreferenceLoader::new(repo_root);

let mut app: App<Box<dyn io::Write + Send>> =
App::new(size.height, size.width, tasks, preferences);
Expand Down Expand Up @@ -933,7 +933,7 @@ mod test {
100,
100,
vec!["foo".to_string(), "bar".to_string(), "baz".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
assert_eq!(
app.task_list_scroll.selected(),
Expand Down Expand Up @@ -980,7 +980,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
assert_eq!(app.task_list_scroll.selected(), Some(1), "selected b");
Expand All @@ -1007,7 +1007,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
app.next();
Expand Down Expand Up @@ -1078,7 +1078,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
app.next();
Expand Down Expand Up @@ -1129,7 +1129,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
assert_eq!(app.task_list_scroll.selected(), Some(1), "selected b");
Expand Down Expand Up @@ -1171,7 +1171,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
assert!(!app.is_focusing_pane(), "app starts focused on table");
app.insert_stdin("a", Some(Vec::new()))?;
Expand Down Expand Up @@ -1202,7 +1202,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
assert_eq!(app.task_list_scroll.selected(), Some(1), "selected b");
Expand All @@ -1228,7 +1228,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
assert_eq!(app.task_list_scroll.selected(), Some(0), "selected a");
assert_eq!(app.tasks_by_status.task_name(0)?, "a", "selected a");
Expand Down Expand Up @@ -1263,7 +1263,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
assert_eq!(app.task_list_scroll.selected(), Some(1), "selected b");
Expand Down Expand Up @@ -1299,7 +1299,7 @@ mod test {
20,
24,
vec!["a".to_string(), "b".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
let pane_rows = app.size.pane_rows();
let pane_cols = app.size.pane_cols();
Expand Down Expand Up @@ -1339,7 +1339,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
app.update_tasks(Vec::new())?;
Expand All @@ -1358,7 +1358,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
app.restart_tasks(vec!["d".to_string()])?;
Expand All @@ -1379,7 +1379,7 @@ mod test {
100,
100,
vec!["a".to_string(), "b".to_string(), "c".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.enter_search()?;
assert!(matches!(app.section_focus, LayoutSections::Search { .. }));
Expand All @@ -1405,7 +1405,7 @@ mod test {
100,
100,
vec!["a".to_string(), "ab".to_string(), "abc".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.enter_search()?;
app.search_enter_char('a')?;
Expand Down Expand Up @@ -1433,7 +1433,7 @@ mod test {
100,
100,
vec!["a".to_string(), "ab".to_string(), "abc".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.enter_search()?;
app.search_enter_char('b')?;
Expand Down Expand Up @@ -1465,7 +1465,7 @@ mod test {
100,
100,
vec!["a".to_string(), "abc".to_string(), "b".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
assert_eq!(app.active_task()?, "abc");
Expand All @@ -1489,7 +1489,7 @@ mod test {
100,
100,
vec!["a".to_string(), "abc".to_string(), "b".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.next();
assert_eq!(app.active_task()?, "abc");
Expand All @@ -1513,7 +1513,7 @@ mod test {
100,
100,
vec!["a".to_string(), "ab".to_string(), "abc".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.enter_search()?;
app.search_enter_char('b')?;
Expand All @@ -1534,7 +1534,7 @@ mod test {
100,
100,
vec!["a".to_string(), "ab".to_string(), "abc".to_string()],
PreferenceLoader::new(&repo_root)?,
PreferenceLoader::new(&repo_root),
);
app.enter_search()?;
app.search_enter_char('b')?;
Expand Down
40 changes: 31 additions & 9 deletions crates/turborepo-ui/src/tui/preferences.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use serde::{Deserialize, Serialize};
use tracing::debug;
use turbopath::AbsoluteSystemPathBuf;

const TUI_PREFERENCES_PATH_COMPONENTS: &[&str] = &[".turbo", "preferences", "tui.json"];
Expand All @@ -17,15 +18,22 @@ pub struct PreferenceLoader {
}

impl PreferenceLoader {
pub fn new(repo_root: &AbsoluteSystemPathBuf) -> Result<Self, Error> {
pub fn new(repo_root: &AbsoluteSystemPathBuf) -> Self {
let file_path = repo_root.join_components(TUI_PREFERENCES_PATH_COMPONENTS);
let contents = file_path.read_existing_to_string()?;
let contents = file_path
.read_existing_to_string()
.map_err(|e| debug!("error reading preferences: {e}"))
.ok()
.flatten();
let config = contents
.map(|string| serde_json::from_str(&string))
.transpose()?
.transpose()
.map_err(|e| debug!("error parsing preferences: {e}"))
.ok()
.flatten()
.unwrap_or_default();

Ok(Self { file_path, config })
Self { file_path, config }
}

pub fn is_task_list_visible(&self) -> bool {
Expand All @@ -41,9 +49,8 @@ impl PreferenceLoader {
Some(active_task)
}

pub fn set_active_task(&mut self, value: Option<String>) -> Result<(), Error> {
pub fn set_active_task(&mut self, value: Option<String>) {
self.config.active_task = value;
Ok(())
}

pub fn flush_to_disk(&self) -> Result<(), Error> {
Expand Down Expand Up @@ -77,7 +84,7 @@ mod test {
use super::*;

fn create_loader(repo_root: AbsoluteSystemPathBuf) -> PreferenceLoader {
PreferenceLoader::new(&repo_root).expect("Failed to create PreferenceLoader")
PreferenceLoader::new(&repo_root)
}

#[test]
Expand Down Expand Up @@ -136,7 +143,7 @@ mod test {
)
.expect("Failed to create file");

let task = PreferenceLoader::new(&repo_root).expect("Failed to create PreferenceLoader");
let task = PreferenceLoader::new(&repo_root);
assert_eq!(task.active_task(), Some("web#dev"));
}

Expand Down Expand Up @@ -166,7 +173,22 @@ mod test {
)
.expect("Failed to create file");

let task = PreferenceLoader::new(&repo_root).expect("Failed to create PreferenceLoader");
let task = PreferenceLoader::new(&repo_root);
assert!(!task.is_task_list_visible());
}

#[test]
fn test_bad_preferences_file_does_not_fail() {
let repo_root_tmp = tempdir().expect("Failed to create tempdir");
let repo_root = AbsoluteSystemPathBuf::try_from(repo_root_tmp.path())
.expect("Failed to create AbsoluteSystemPathBuf");
let preference_path = repo_root.join_components(TUI_PREFERENCES_PATH_COMPONENTS);
preference_path.ensure_dir().unwrap();
preference_path
.create_with_contents("this isn't json!")
.unwrap();

let loader = create_loader(repo_root);
assert!(loader.active_task().is_none());
}
}

0 comments on commit 3e774d3

Please sign in to comment.