fix+refactor: link handling (#292)

* feat+fix+refactor

Now we set the current directory to the directory of the file so that relative link can be opened by open::that, also added an error message when open::that fails.

* clippy

* fix: Removed edge case where opening an relative file would result in crashing the file watcher.

* fmt

* test: Use a tempdir for history's test env

* test: Use a tempfile where needed in CLI test env

* fix: Canonicalize the tempdir as well

* chore: Dont check for typos on test data

---------

Co-authored-by: Cosmic Horror <CosmicHorrorDev@pm.me>

src/history.rs

@@ -7,9 +7,10 @@ pub struct History {
 }
 
 impl History {
- pub fn new(path_buf: PathBuf) -> Self {
+ pub fn new(path: &Path) -> Self {
+ let canonicalized = path.canonicalize().unwrap();
  Self {
- history: vec![path_buf],
+ history: vec![canonicalized],
  index: 0,
  }
  }
@@ -22,6 +23,8 @@ impl History {
  }
 
  pub fn make_next(&mut self, file_path: PathBuf) {
+ let file_path = file_path.canonicalize().unwrap();
+
  self.history.truncate(self.index + 1);
  self.history.push(file_path);
  self.index += 1;
@@ -49,15 +52,26 @@ impl History {
 
 #[cfg(test)]
 mod tests {
+ use std::fs;
+
  use super::*;
 
  #[test]
  fn sanity() {
- let root = PathBuf::from("a");
- let fork1 = PathBuf::from("b");
- let fork2 = PathBuf::from("c");
+ let temp_dir = tempfile::Builder::new()
+ .prefix("inlyne-tests-")
+ .tempdir()
+ .unwrap();
+ let temp_path = temp_dir.path().canonicalize().unwrap();
+
+ let root = temp_path.join("a");
+ let fork1 = temp_path.join("b");
+ let fork2 = temp_path.join("c");
+ fs::write(&root, "a").unwrap();
+ fs::write(&fork1, "b").unwrap();
+ fs::write(&fork2, "c").unwrap();
 
- let mut hist = History::new(root.clone());
+ let mut hist = History::new(&root);
  assert_eq!(hist.get_path(), root);
  assert_eq!(hist.previous(), None);
 

src/main.rs

@@ -205,6 +205,8 @@ impl Inlyne {
 
  let watcher = Watcher::spawn(event_loop.create_proxy(), file_path.clone());
 
+ let _ = file_path.parent().map(std::env::set_current_dir);
+
  Ok(Self {
  opts,
  window,
@@ -455,89 +457,57 @@ impl Inlyne {
  screen_size,
  self.renderer.zoom,
  ) {
- if let Hoverable::Summary(summary) = hoverable {
- let mut hidden = summary.hidden.borrow_mut();
- *hidden = !*hidden;
- event_loop_proxy
- .send_event(InlyneEvent::Reposition)
- .unwrap();
- }
-
- let maybe_link = match hoverable {
- Hoverable::Image(Image { is_link, .. }) => is_link,
- Hoverable::Text(Text { link, .. }) => link,
- Hoverable::Summary(_) => &None,
- };
-
- if let Some(link) = maybe_link {
- let maybe_path = PathBuf::from_str(link).ok();
- let is_local_md = maybe_path.as_ref().map_or(false, |p| {
- p.extension().map_or(false, |ext| ext == "md")
- && !p.to_str().map_or(false, |s| s.starts_with("http"))
- });
- if is_local_md {
- // Open markdown files ourselves
- let path = maybe_path.expect("not a path");
- // Handle relative paths and make them
- // absolute by prepending current
- // parent
- let path = if path.is_relative() {
- // Simply canonicalizing it doesn't suffice and leads to "no such file or directory"
- let current_parent = self
- .opts
- .history
- .get_path()
- .parent()
- .expect("no current parent");
- let mut normalized_link = path.as_path();
- if let Ok(stripped) = normalized_link
- .strip_prefix(std::path::Component::CurDir)
- {
- normalized_link = stripped;
- }
- let mut link = current_parent.to_path_buf();
- link.push(normalized_link);
- link
- } else {
- path
- };
- // Open them in a new window, akin to what a browser does
- if modifiers.shift() {
- Command::new(
- std::env::current_exe()
- .unwrap_or_else(|_| "inlyne".into()),
- )
- .args(Opts::program_args(&path))
- .spawn()
- .expect("Could not spawn new inlyne instance");
- } else {
- match read_to_string(&path) {
- Ok(contents) => {
- self.update_file(&path, contents);
- self.opts.history.make_next(path);
- }
- Err(err) => {
- tracing::warn!(
+ match hoverable {
+ Hoverable::Image(Image { is_link: Some(link), .. }) |
+ Hoverable::Text(Text { link: Some(link), .. }) => {
+ let path = PathBuf::from_str(link).unwrap(); // Can't fail
+
+ if path.extension().map_or(false, |ext| ext == "md")
+ && !path.to_str().map_or(false, |s| s.starts_with("http")) {
+ // Open them in a new window, akin to what a browser does
+ if modifiers.shift() {
+ Command::new(
+ std::env::current_exe()
+ .unwrap_or_else(|_| "inlyne".into()),
+ )
+ .args(Opts::program_args(&path))
+ .spawn()
+ .expect("Could not spawn new inlyne instance");
+ } else {
+ match read_to_string(&path) {
+ Ok(contents) => {
+ self.update_file(&path, contents);
+ self.opts.history.make_next(path);
+ }
+ Err(err) => {
+ tracing::warn!(
  "Failed loading markdown file at {}\nError: {}",
  path.display(),
  err,
  );
+ }
  }
  }
+ } else if let Some(anchor_pos) =
+ self.renderer.positioner.anchors.get(&link.to_lowercase())
+ {
+ self.renderer.set_scroll_y(*anchor_pos);
+ self.window.request_redraw();
+ self.window.set_cursor_icon(CursorIcon::Default);
+ } else if let Err(e) = open::that(link) {
+ tracing::error!("Could not open link: {e} from {:?}", std::env::current_dir())
  }
- } else if let Some(anchor_pos) =
- self.renderer.positioner.anchors.get(&link.to_lowercase())
- {
- self.renderer.set_scroll_y(*anchor_pos);
- self.window.request_redraw();
- self.window.set_cursor_icon(CursorIcon::Default);
- } else {
- open::that(link).unwrap();
- }
- } else if self.renderer.selection.is_none() {
- // Only set selection when not over link
- self.renderer.selection = Some((last_loc, last_loc));
- }
+ },
+ Hoverable::Summary(summary) => {
+ let mut hidden = summary.hidden.borrow_mut();
+ *hidden = !*hidden;
+ event_loop_proxy
+ .send_event(InlyneEvent::Reposition)
+ .unwrap();
+ self.renderer.selection = Some((last_loc, last_loc))
+ },
+ _ => self.renderer.selection = Some((last_loc, last_loc)),
+ };
  } else if self.renderer.selection.is_none() {
  self.renderer.selection = Some((last_loc, last_loc));
  }
@@ -630,6 +600,8 @@ impl Inlyne {
  match read_to_string(&file_path) {
  Ok(contents) => {
  self.update_file(&file_path, contents);
+ let parent = file_path.parent().expect("File should have parent directory");
+ std::env::set_current_dir(parent).expect("Could not set current directory.");
  }
  Err(err) => {
  tracing::warn!(

src/opts/mod.rs

@@ -138,7 +138,7 @@ impl Opts {
  };
 
  Ok(Self {
- history: History::new(file_path),
+ history: History::new(&file_path),
  theme,
  scale,
  page_width,