Add source information as path of the information #2466
Conversation
|
Seems reasonable to me! Would it be better to have relative paths or absolute paths? |
Yeah, I think absolute paths would be more convenient. And it was trivial to implement it. Let me see how to make the tests pass using that. |
Motivation: To add support for module integration in justl which is an Emacs extension for driving justfiles within Emacs. This would allow me to easily discover the file locations of different just modules and allow the ability to individually open justl buffers for them within the editor.
|
@casey This is ready for review now. |
src/analyzer.rs
Outdated
| @@ -182,6 +182,7 @@ impl<'run, 'src> Analyzer<'run, 'src> { | |||
| unstable_features.insert(UnstableFeature::ScriptInterpreterSetting); | |||
| } | |||
|
|
|||
| let source = root.canonicalize().unwrap(); | |||
There was a problem hiding this comment.
Canonicalize produces weird paths. On Unix, it resolves symlinks, which is often not what you want, and on Windows it produces paths prefixed with \\?\ which is not very nice.
Canonicalize is also a system call, which means it can fail with an I/O error, so we can't unwrap here.
If we need to clean the path, we should use .lexiclean() instead of canonicalize, which cannot fail and doesn't produce weird paths.
There was a problem hiding this comment.
I have pushed a fix, but the tests in MacOS fails because I think of symbolic link. Do you know how to handle it ?
There was a problem hiding this comment.
Okay, I have pushed a fix where I have to use canonicalize only for the tests to make it pass. Let me know what you think.
tests/json.rs
Outdated
| @@ -1,22 +1,37 @@ | |||
| use super::*; | |||
|
|
|||
| fn case(justfile: &str, value: Value) { | |||
| fn case<F: Fn(&Path) -> Value>(justfile: &str, value: F) { | |||
There was a problem hiding this comment.
The JSON tests are pretty horrible. I changed the case function so that you pass in a struct which should serialize to the output of --dump.
You can take advantage of this by putting the relative path in the source field of the passed in Module, and then, in case, prepending the path to the temporary directory, which will make them match.
|
Thank you! |
Motivation: To add support for module integration in justl which is an Emacs extension for driving justfiles within Emacs.
This would allow me to easily discover the file locations of different just modules and allow the ability to individually open justl buffers for them within the editor.
More info here: