Skip to content

Commit

Permalink
feat: let filenames in errors be relative to the current dir if possi…
Browse files Browse the repository at this point in the history
…ble (noir-lang#5642)

# Description

## Problem

Resolves noir-lang#5641

## Summary

It seems `FileMap`'s `name` function was the one that needed that
change.

To avoid doing a syscall to get the current directory every time a name
is needed, `FileMap` gets it at the beginning of the program. I think
this is fine as the current directory can't change while the compilation
is happening.

## Additional Context

None.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
asterite authored Jul 30, 2024
1 parent 80128ff commit f656681
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
18 changes: 16 additions & 2 deletions compiler/fm/src/file_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ impl From<&PathBuf> for PathString {
pub struct FileMap {
files: SimpleFiles<PathString, String>,
name_to_id: HashMap<PathString, FileId>,
current_dir: Option<PathBuf>,
}

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
Expand Down Expand Up @@ -82,7 +83,11 @@ impl FileMap {
}
impl Default for FileMap {
fn default() -> Self {
FileMap { files: SimpleFiles::new(), name_to_id: HashMap::new() }
FileMap {
files: SimpleFiles::new(),
name_to_id: HashMap::new(),
current_dir: std::env::current_dir().ok(),
}
}
}

Expand All @@ -92,7 +97,16 @@ impl<'a> Files<'a> for FileMap {
type Source = &'a str;

fn name(&self, file_id: Self::FileId) -> Result<Self::Name, Error> {
Ok(self.files.get(file_id.as_usize())?.name().clone())
let name = self.files.get(file_id.as_usize())?.name().clone();

// See if we can make the file name a bit shorter/easier to read if it starts with the current directory
if let Some(current_dir) = &self.current_dir {
if let Ok(name_without_prefix) = name.0.strip_prefix(current_dir) {
return Ok(PathString::from_path(name_without_prefix.to_path_buf()));
}
}

Ok(name)
}

fn source(&'a self, file_id: Self::FileId) -> Result<Self::Source, Error> {
Expand Down
8 changes: 7 additions & 1 deletion tooling/lsp/src/requests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::path::PathBuf;
use std::{collections::HashMap, future::Future};

use crate::insert_all_files_for_workspace_into_file_manager;
Expand Down Expand Up @@ -324,7 +325,12 @@ where
let file_name = files.name(file_id).ok()?;

let path = file_name.to_string();
let uri = Url::from_file_path(path).ok()?;

// `path` might be a relative path so we canonicalize it to get an absolute path
let path_buf = PathBuf::from(path);
let path_buf = path_buf.canonicalize().unwrap_or(path_buf);

let uri = Url::from_file_path(path_buf.to_str()?).ok()?;

Some(Location { uri, range })
}
Expand Down

0 comments on commit f656681

Please sign in to comment.