Repository: casey/just
Pull request #2466 (2024-11-16 12:16:07)

Add source information as path of the information
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:
- https://github.com/casey/just/issues/2252#issuecomment-2474171211
- https://github.com/psibi/justl.el/issues/57

casey: Seems reasonable to me! Would it be better to have relative paths or absolute paths?
psibi: > 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.


psibi: @casey  This is ready for review now.

Patch:
diff --git a/src/analyzer.rs b/src/analyzer.rs
index ff5dd187..1087bdee 100644
--- a/src/analyzer.rs
+++ b/src/analyzer.rs
@@ -184,6 +184,7 @@ impl<'run, 'src> Analyzer<'run, 'src> {
       unstable_features.insert(UnstableFeature::ScriptInterpreterSetting);
     }
 
+    let source = root.to_owned();
     let root = paths.get(root).unwrap();
 
     Ok(Justfile {
@@ -207,7 +208,7 @@ impl<'run, 'src> Analyzer<'run, 'src> {
       name,
       recipes,
       settings,
-      source: root.into(),
+      source,
       unexports: self.unexports,
       unstable_features,
       warnings: self.warnings,
diff --git a/src/function.rs b/src/function.rs
index 104aeb02..e5e93632 100644
--- a/src/function.rs
+++ b/src/function.rs
@@ -447,50 +447,23 @@ fn lowercase(_context: Context, s: &str) -> FunctionResult {
 }
 
 fn module_directory(context: Context) -> FunctionResult {
-  context
-    .evaluator
-    .context
-    .search
-    .justfile
-    .parent()
-    .unwrap()
-    .join(&context.evaluator.context.module.source)
-    .parent()
-    .unwrap()
-    .to_str()
-    .map(str::to_owned)
-    .ok_or_else(|| {
-      format!(
-        "Module directory is not valid unicode: {}",
-        context
-          .evaluator
-          .context
-          .module
-          .source
-          .parent()
-          .unwrap()
-          .display(),
-      )
-    })
+  let module_directory = context.evaluator.context.module.source.parent().unwrap();
+  module_directory.to_str().map(str::to_owned).ok_or_else(|| {
+    format!(
+      "Module directory is not valid unicode: {}",
+      module_directory.display(),
+    )
+  })
 }
 
 fn module_file(context: Context) -> FunctionResult {
-  context
-    .evaluator
-    .context
-    .search
-    .justfile
-    .parent()
-    .unwrap()
-    .join(&context.evaluator.context.module.source)
-    .to_str()
-    .map(str::to_owned)
-    .ok_or_else(|| {
-      format!(
-        "Module file path is not valid unicode: {}",
-        context.evaluator.context.module.source.display(),
-      )
-    })
+  let module_file = &context.evaluator.context.module.source;
+  module_file.to_str().map(str::to_owned).ok_or_else(|| {
+    format!(
+      "Module file path is not valid unicode: {}",
+      module_file.display(),
+    )
+  })
 }
 
 fn num_cpus(_context: Context) -> FunctionResult {
diff --git a/src/justfile.rs b/src/justfile.rs
index 4af175c1..2ce96ece 100644
--- a/src/justfile.rs
+++ b/src/justfile.rs
@@ -23,7 +23,6 @@ pub(crate) struct Justfile<'src> {
   pub(crate) name: Option<Name<'src>>,
   pub(crate) recipes: Table<'src, Rc<Recipe<'src>>>,
   pub(crate) settings: Settings<'src>,
-  #[serde(skip)]
   pub(crate) source: PathBuf,
   pub(crate) unexports: HashSet<String>,
   #[serde(skip)]


Test patch:
diff --git a/tests/json.rs b/tests/json.rs
index a0fa9f6e..4e740e52 100644
--- a/tests/json.rs
+++ b/tests/json.rs
@@ -31,7 +31,7 @@ struct Interpreter<'a> {
   command: &'a str,
 }
 
-#[derive(Debug, Default, PartialEq, Serialize)]
+#[derive(Debug, Default, PartialEq, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 struct Module<'a> {
   aliases: BTreeMap<&'a str, Alias<'a>>,
@@ -44,6 +44,7 @@ struct Module<'a> {
   settings: Settings<'a>,
   unexports: Vec<&'a str>,
   warnings: Vec<&'a str>,
+  source: PathBuf,
 }
 
 #[derive(Debug, Default, Deserialize, PartialEq, Serialize)]
@@ -98,8 +99,26 @@ fn case(justfile: &str, expected: Module) {
   case_with_submodule(justfile, None, expected);
 }
 
+fn fix_source(dir: &Path, module: &mut Module) {
+  let filename = if module.source.as_os_str().is_empty() {
+    Path::new("justfile")
+  } else {
+    &module.source
+  };
+
+  module.source = if cfg!(target_os = "macos") {
+    dir.canonicalize().unwrap().join(filename)
+  } else {
+    dir.join(filename)
+  };
+
+  for module in module.modules.values_mut() {
+    fix_source(dir, module);
+  }
+}
+
 #[track_caller]
-fn case_with_submodule(justfile: &str, submodule: Option<(&str, &str)>, expected: Module) {
+fn case_with_submodule(justfile: &str, submodule: Option<(&str, &str)>, mut expected: Module) {
   let mut test = Test::new()
     .justfile(justfile)
     .args(["--dump", "--dump-format", "json"])
@@ -109,11 +128,11 @@ fn case_with_submodule(justfile: &str, submodule: Option<(&str, &str)>, expected
     test = test.write(path, source);
   }
 
-  let actual = test.run().stdout;
+  fix_source(test.tempdir.path(), &mut expected);
 
-  let mut expected = serde_json::to_string(&expected).unwrap();
-  expected.push('\n');
+  let actual = test.run().stdout;
 
+  let actual: Module = serde_json::from_str(actual.as_str()).unwrap();
   pretty_assertions::assert_eq!(actual, expected);
 }
 
@@ -776,6 +795,7 @@ fn module() {
         Module {
           doc: Some("hello"),
           first: Some("bar"),
+          source: "foo.just".into(),
           recipes: [(
             "bar",
             Recipe {
@@ -808,6 +828,7 @@ fn module_group() {
         Module {
           first: Some("bar"),
           groups: ["alpha"].into(),
+          source: "foo.just".into(),
           recipes: [(
             "bar",
             Recipe {