diff --git a/helix-loader/src/persistence.rs b/helix-loader/src/persistence.rs index c9ec96871..019977794 100644 --- a/helix-loader/src/persistence.rs +++ b/helix-loader/src/persistence.rs @@ -12,11 +12,9 @@ pub fn write_history(filepath: PathBuf, entries: &Vec) { .create(true) .truncate(true) .open(filepath) - // TODO: do something about this unwrap .unwrap(); for entry in entries { - // TODO: do something about this unwrap serialize_into(&file, &entry).unwrap(); } } @@ -26,19 +24,23 @@ pub fn push_history(filepath: PathBuf, entry: &T) { .append(true) .create(true) .open(filepath) - // TODO: do something about this unwrap .unwrap(); - // TODO: do something about this unwrap serialize_into(file, entry).unwrap(); } -pub fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { +pub fn read_history Deserialize<'a>>(filepath: &PathBuf) -> Vec { match File::open(filepath) { Ok(file) => { let mut read = BufReader::new(file); let mut entries = Vec::new(); - // TODO: more sophisticated error handling + // FIXME: Can we do better error handling here? It's unfortunate that bincode doesn't + // distinguish an empty reader from an actual error. + // + // Perhaps we could use the underlying bufreader to check for emptiness in the while + // condition, then we could know any errors from bincode should be surfaced or logged. + // BufRead has a method `has_data_left` that would work for this, but at the time of + // writing it is nightly-only and experimental :( while let Ok(entry) = deserialize_from(&mut read) { entries.push(entry); } @@ -46,8 +48,13 @@ pub fn read_history Deserialize<'a>>(filepath: PathBuf) -> Vec { } Err(e) => match e.kind() { io::ErrorKind::NotFound => Vec::new(), - // TODO: do something about this panic - _ => panic!(), + // Going through the potential errors listed from the docs: + // - `InvalidInput` can't happen since we aren't setting options + // - `AlreadyExists` can't happen since we aren't setting `create_new` + // - `PermissionDenied` could happen if someone really borked their file permissions + // in `~/.local`, but helix already panics in that case, and I think a panic is + // acceptable. + _ => unreachable!(), }, } } @@ -56,8 +63,7 @@ pub fn trim_history Deserialize<'a>>( filepath: PathBuf, limit: usize, ) { - // TODO: can we remove this clone? - let history: Vec = read_history(filepath.clone()); + let history: Vec = read_history(&filepath); if history.len() > limit { let trim_start = history.len() - limit; let trimmed_history = history[trim_start..].to_vec(); diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index be2e35cb4..ab7ce1237 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -164,26 +164,23 @@ pub fn new(args: Args, config: Config, lang_loader: syntax::Loader) -> Result Result Action::HorizontalSplit, None => Action::Load, }; - let doc_id = match editor.open(&file, action) { + match editor.open(&file, action) { // Ignore irregular files during application init. Err(DocumentOpenError::IrregularFile) => { nr_of_files -= 1; continue; } Err(err) => return Err(anyhow::anyhow!(err)), - Ok(doc_id) => doc_id, + Ok(_) => (), }; // with Action::Load all documents have the same view // NOTE: this isn't necessarily true anymore. If // `--vsplit` or `--hsplit` are used, the file which is // opened last is focused on. if let Some(pos) = pos { - let view_id = editor.tree.focus; - let doc = doc_mut!(editor, &doc_id); + let (view, doc) = current!(editor); let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); - doc.set_selection(view_id, pos); + doc.set_selection(view.id, pos); + align_view(doc, view, Align::Center); } } } @@ -260,10 +257,6 @@ pub fn new(args: Args, config: Config, lang_loader: syntax::Loader) -> Result], event: PromptEvent) -> if let Some(pos) = pos { let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); doc.set_selection(view.id, pos); + align_view(doc, view, Align::Center); } - // does not affect opening a buffer without pos - // TODO: ensure removing this will not cause problems - // align_view(doc, view, Align::Center); } } Ok(()) diff --git a/helix-term/tests/test/persistence.rs b/helix-term/tests/test/persistence.rs index 34bacb426..ac244ecda 100644 --- a/helix-term/tests/test/persistence.rs +++ b/helix-term/tests/test/persistence.rs @@ -53,11 +53,7 @@ async fn test_persistence() -> anyhow::Result<()> { .with_config(config_with_persistence()) .with_file(file.path(), None) .build()?, - // TODO: remove the h with a bugfix? - Some("oah:wq"), - // Some(&|app| { - // assert!(!app.editor.is_err(), "error: {:?}", app.editor.get_status()); - // }), + Some("oa:wq"), None, true, ) @@ -69,10 +65,10 @@ async fn test_persistence() -> anyhow::Result<()> { // Session 2: // open same file, // add newline, then b, - // copy the line + // copy the line ("b\n") // search for "a" // go back down to b - // use last command + // use last command (write-quit) test_key_sequence( &mut helpers::AppBuilder::new() .with_config(config_with_persistence()) @@ -91,10 +87,10 @@ async fn test_persistence() -> anyhow::Result<()> { // Session 3: // open same file, // paste - // use last search + // use last search ("/a") // append a // search for "1", "2", and "3" in sequence. - // use last command + // use last command (write-quit) test_key_sequence( &mut helpers::AppBuilder::new() .with_config(config_with_persistence()) @@ -112,7 +108,7 @@ async fn test_persistence() -> anyhow::Result<()> { // Session 4: // open same file - // use last command + // use last command (write-quit) test_key_sequence( &mut helpers::AppBuilder::new() .with_config(config_with_persistence()) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c7add0994..1b25b1e0f 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1740,14 +1740,9 @@ pub fn open(&mut self, path: &Path, action: Action) -> Result Result Result= doc_len) { + // Don't restore the view and selection if the selection goes beyond the file's end + if !selection.ranges().iter().any(|range| range.to() > doc_len) { doc.set_view_offset(view.id, view_position); doc.set_selection(view.id, selection); } diff --git a/helix-view/src/persistence.rs b/helix-view/src/persistence.rs index 332fc0392..754b08aa4 100644 --- a/helix-view/src/persistence.rs +++ b/helix-view/src/persistence.rs @@ -31,7 +31,7 @@ pub fn push_file_history(entry: &FileHistoryEntry) { } pub fn read_file_history() -> Vec { - read_history(file_histfile()) + read_history(&file_histfile()) } pub fn trim_file_history(limit: usize) { @@ -49,7 +49,7 @@ pub fn push_reg_history(register: char, line: &String) { } fn read_reg_history(filepath: PathBuf) -> Vec { - read_history(filepath) + read_history(&filepath) } pub fn read_command_history() -> Vec { @@ -77,5 +77,5 @@ pub fn write_clipboard_file(values: &Vec) { } pub fn read_clipboard_file() -> Vec { - read_history(clipboard_file()) + read_history(&clipboard_file()) }