Give modules doc comments for --list (#2199)

src/analyzer.rs

@@ -9,22 +9,24 @@ pub(crate) struct Analyzer<'src> {
 
 impl<'src> Analyzer<'src> {
   pub(crate) fn analyze(
-    loaded: &[PathBuf],
-    paths: &HashMap<PathBuf, PathBuf>,
     asts: &HashMap<PathBuf, Ast<'src>>,
-    root: &Path,
+    doc: Option<&'src str>,
+    loaded: &[PathBuf],
     name: Option<Name<'src>>,
+    paths: &HashMap<PathBuf, PathBuf>,
+    root: &Path,
   ) -> CompileResult<'src, Justfile<'src>> {
-    Self::default().justfile(loaded, paths, asts, root, name)
+    Self::default().justfile(asts, doc, loaded, name, paths, root)
   }
 
   fn justfile(
     mut self,
-    loaded: &[PathBuf],
-    paths: &HashMap<PathBuf, PathBuf>,
     asts: &HashMap<PathBuf, Ast<'src>>,
-    root: &Path,
+    doc: Option<&'src str>,
+    loaded: &[PathBuf],
     name: Option<Name<'src>>,
+    paths: &HashMap<PathBuf, PathBuf>,
+    root: &Path,
   ) -> CompileResult<'src, Justfile<'src>> {
     let mut recipes = Vec::new();
 
@@ -84,10 +86,22 @@ impl<'src> Analyzer<'src> {
               stack.push(asts.get(absolute).unwrap());
             }
           }
-          Item::Module { absolute, name, .. } => {
+          Item::Module {
+            absolute,
+            name,
+            doc,
+            ..
+          } => {
             if let Some(absolute) = absolute {
               define(*name, "module", false)?;
-              modules.insert(Self::analyze(loaded, paths, asts, absolute, Some(*name))?);
+              modules.insert(Self::analyze(
+                asts,
+                *doc,
+                loaded,
+                Some(*name),
+                paths,
+                absolute,
+              )?);
             }
           }
           Item::Recipe(recipe) => {
@@ -172,6 +186,7 @@ impl<'src> Analyzer<'src> {
             Rc::clone(next)
           }),
         }),
+      doc,
       loaded: loaded.into(),
       modules,
       name,

src/compiler.rs

@@ -40,6 +40,7 @@ impl Compiler {
             name,
             optional,
             relative,
+            ..
           } => {
             if !unstable {
               return Err(Error::Unstable {
@@ -107,7 +108,7 @@ impl Compiler {
       asts.insert(current.path, ast.clone());
     }
 
-    let justfile = Analyzer::analyze(&loaded, &paths, &asts, root, None)?;
+    let justfile = Analyzer::analyze(&asts, None, &loaded, None, &paths, root)?;
 
     Ok(Compilation {
       asts,
@@ -184,7 +185,7 @@ impl Compiler {
     asts.insert(root.clone(), ast);
     let mut paths: HashMap<PathBuf, PathBuf> = HashMap::new();
     paths.insert(root.clone(), root.clone());
-    Analyzer::analyze(&[], &paths, &asts, &root, None)
+    Analyzer::analyze(&asts, None, &[], None, &paths, &root)
   }
 }
 

src/item.rs

@@ -14,6 +14,7 @@ pub(crate) enum Item<'src> {
   },
   Module {
     absolute: Option<PathBuf>,
+    doc: Option<&'src str>,
     name: Name<'src>,
     optional: bool,
     relative: Option<StringLiteral<'src>>,

src/justfile.rs

@@ -13,6 +13,7 @@ struct Invocation<'src: 'run, 'run> {
 pub(crate) struct Justfile<'src> {
   pub(crate) aliases: Table<'src, Alias<'src>>,
   pub(crate) assignments: Table<'src, Assignment<'src>>,
+  pub(crate) doc: Option<&'src str>,
   #[serde(rename = "first", serialize_with = "keyed::serialize_option")]
   pub(crate) default: Option<Rc<Recipe<'src>>>,
   #[serde(skip)]

src/parser.rs

@@ -372,6 +372,8 @@ impl<'run, 'src> Parser<'run, 'src> {
               || self.next_are(&[Identifier, Identifier, StringToken])
               || self.next_are(&[Identifier, QuestionMark]) =>
           {
+            let doc = pop_doc_comment(&mut items, eol_since_last_comment);
+
             self.presume_keyword(Keyword::Mod)?;
 
             let optional = self.accepted(QuestionMark)?;
@@ -387,6 +389,7 @@ impl<'run, 'src> Parser<'run, 'src> {
 
             items.push(Item::Module {
               absolute: None,
+              doc,
               name,
               optional,
               relative,

src/subcommand.rs

@@ -435,6 +435,27 @@ impl Subcommand {
   }
 
   fn list_module(config: &Config, module: &Justfile, depth: usize) {
+    fn format_doc(
+      config: &Config,
+      name: &str,
+      doc: Option<&str>,
+      max_signature_width: usize,
+      signature_widths: &BTreeMap<&str, usize>,
+    ) {
+      if let Some(doc) = doc {
+        if doc.lines().count() <= 1 {
+          print!(
+            "{:padding$}{} {}",
+            "",
+            config.color.stdout().doc().paint("#"),
+            config.color.stdout().doc().paint(doc),
+            padding = max_signature_width.saturating_sub(signature_widths[name]) + 1,
+          );
+        }
+      }
+      println!();
+    }
+
     let aliases = if config.no_aliases {
       BTreeMap::new()
     } else {
@@ -468,6 +489,11 @@ impl Subcommand {
           );
         }
       }
+      if !config.list_submodules {
+        for (name, _) in &module.modules {
+          signature_widths.insert(name, UnicodeWidthStr::width(format!("{name} ...").as_str()));
+        }
+      }
 
       signature_widths
     };
@@ -554,20 +580,15 @@ impl Subcommand {
             RecipeSignature { name, recipe }.color_display(config.color.stdout())
           );
 
-          if let Some(doc) = doc {
+          format_doc(
-            if doc.lines().count() <= 1 {
+            config,
-              print!(
+            name,
-                "{:padding$}{} {}",
+            doc.as_deref(),
-                "",
+            max_signature_width,
-                config.color.stdout().doc().paint("#"),
+            &signature_widths,
-                config.color.stdout().doc().paint(&doc),
-                padding = max_signature_width.saturating_sub(signature_widths[name]) + 1,
           );
         }
       }
-          println!();
-        }
-      }
     }
 
     if config.list_submodules {
@@ -582,7 +603,14 @@ impl Subcommand {
       }
     } else {
       for submodule in module.modules(config) {
-        println!("{list_prefix}{} ...", submodule.name(),);
+        print!("{list_prefix}{} ...", submodule.name());
+        format_doc(
+          config,
+          submodule.name(),
+          submodule.doc,
+          max_signature_width,
+          &signature_widths,
+        );
       }
     }
   }

src/testing.rs

@@ -77,7 +77,7 @@ pub(crate) fn analysis_error(
   let mut paths: HashMap<PathBuf, PathBuf> = HashMap::new();
   paths.insert("justfile".into(), "justfile".into());
 
-  match Analyzer::analyze(&[], &paths, &asts, &root, None) {
+  match Analyzer::analyze(&asts, None, &[], None, &paths, &root) {
     Ok(_) => panic!("Analysis unexpectedly succeeded"),
     Err(have) => {
       let want = CompileError {

tests/functions.rs

@@ -34,10 +34,10 @@ b := env_var_or_default('ZADDY', 'HTAP')
 x := env_var_or_default('XYZ', 'ABC')
 
 foo:
-  /bin/echo '{{p}}' '{{b}}' '{{x}}'
+  /usr/bin/env echo '{{p}}' '{{b}}' '{{x}}'
 "#,
   stdout:   format!("{} HTAP ABC\n", env::var("USER").unwrap()).as_str(),
-  stderr:   format!("/bin/echo '{}' 'HTAP' 'ABC'\n", env::var("USER").unwrap()).as_str(),
+  stderr:   format!("/usr/bin/env echo '{}' 'HTAP' 'ABC'\n", env::var("USER").unwrap()).as_str(),
 }
 
 #[cfg(not(windows))]
@@ -52,10 +52,10 @@ ext := extension('/foo/bar/baz.hello')
 jn  := join('a', 'b')
 
 foo:
-  /bin/echo '{{we}}' '{{fs}}' '{{fn}}' '{{dir}}' '{{ext}}' '{{jn}}'
+  /usr/bin/env echo '{{we}}' '{{fs}}' '{{fn}}' '{{dir}}' '{{ext}}' '{{jn}}'
 "#,
   stdout:   "/foo/bar/baz baz baz.hello /foo/bar hello a/b\n",
-  stderr:   "/bin/echo '/foo/bar/baz' 'baz' 'baz.hello' '/foo/bar' 'hello' 'a/b'\n",
+  stderr:   "/usr/bin/env echo '/foo/bar/baz' 'baz' 'baz.hello' '/foo/bar' 'hello' 'a/b'\n",
 }
 
 #[cfg(not(windows))]
@@ -69,10 +69,10 @@ dir := parent_directory('/foo/')
 ext := extension('/foo/bar/baz.hello.ciao')
 
 foo:
-  /bin/echo '{{we}}' '{{fs}}' '{{fn}}' '{{dir}}' '{{ext}}'
+  /usr/bin/env echo '{{we}}' '{{fs}}' '{{fn}}' '{{dir}}' '{{ext}}'
 "#,
   stdout:   "/foo/bar/baz baz.hello baz.hello.ciao / ciao\n",
-  stderr:   "/bin/echo '/foo/bar/baz' 'baz.hello' 'baz.hello.ciao' '/' 'ciao'\n",
+  stderr:   "/usr/bin/env echo '/foo/bar/baz' 'baz.hello' 'baz.hello.ciao' '/' 'ciao'\n",
 }
 
 #[cfg(not(windows))]
@@ -82,7 +82,7 @@ test! {
 we  := without_extension('')
 
 foo:
-  /bin/echo '{{we}}'
+  /usr/bin/env echo '{{we}}'
 "#,
   stdout:   "",
   stderr:   format!("{} {}\n{}\n{}\n{}\n{}\n",
@@ -102,7 +102,7 @@ test! {
 we  := extension('')
 
 foo:
-  /bin/echo '{{we}}'
+  /usr/bin/env echo '{{we}}'
 "#,
   stdout:   "",
   stderr:   format!("{}\n{}\n{}\n{}\n{}\n",
@@ -121,7 +121,7 @@ test! {
 we  := extension('foo')
 
 foo:
-  /bin/echo '{{we}}'
+  /usr/bin/env echo '{{we}}'
 "#,
   stdout:   "",
   stderr:   format!("{}\n{}\n{}\n{}\n{}\n",
@@ -140,7 +140,7 @@ test! {
 we  := file_stem('')
 
 foo:
-  /bin/echo '{{we}}'
+  /usr/bin/env echo '{{we}}'
 "#,
   stdout:   "",
   stderr:   format!("{}\n{}\n{}\n{}\n{}\n",
@@ -159,7 +159,7 @@ test! {
 we  := file_name('')
 
 foo:
-  /bin/echo '{{we}}'
+  /usr/bin/env echo '{{we}}'
 "#,
   stdout:   "",
   stderr:   format!("{}\n{}\n{}\n{}\n{}\n",
@@ -178,7 +178,7 @@ test! {
 we  := parent_directory('')
 
 foo:
-  /bin/echo '{{we}}'
+  /usr/bin/env echo '{{we}}'
 "#,
   stdout:   "",
   stderr:   format!("{} {}\n{}\n{}\n{}\n{}\n",
@@ -198,7 +198,7 @@ test! {
 we  := parent_directory('/')
 
 foo:
-  /bin/echo '{{we}}'
+  /usr/bin/env echo '{{we}}'
 "#,
   stdout:   "",
   stderr:   format!("{} {}\n{}\n{}\n{}\n{}\n",
@@ -220,10 +220,10 @@ b := env_var_or_default('ZADDY', 'HTAP')
 x := env_var_or_default('XYZ', 'ABC')
 
 foo:
-  /bin/echo '{{p}}' '{{b}}' '{{x}}'
+  /usr/bin/env echo '{{p}}' '{{b}}' '{{x}}'
 "#,
   stdout:   format!("{} HTAP ABC\n", env::var("USERNAME").unwrap()).as_str(),
-  stderr:   format!("/bin/echo '{}' 'HTAP' 'ABC'\n", env::var("USERNAME").unwrap()).as_str(),
+  stderr:   format!("/usr/bin/env echo '{}' 'HTAP' 'ABC'\n", env::var("USERNAME").unwrap()).as_str(),
 }
 
 test! {

tests/json.rs

@@ -18,6 +18,7 @@ fn alias() {
     ",
     json!({
       "first": "foo",
+      "doc": null,
       "aliases": {
         "f": {
           "name": "f",
@@ -80,6 +81,7 @@ fn assignment() {
         }
       },
       "first": null,
+      "doc": null,
       "modules": {},
       "recipes": {},
       "settings": {
@@ -117,6 +119,7 @@ fn body() {
       "aliases": {},
       "assignments": {},
       "first": "foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "foo": {
@@ -170,6 +173,7 @@ fn dependencies() {
       "aliases": {},
       "assignments": {},
       "first": "foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "bar": {
@@ -248,6 +252,7 @@ fn dependency_argument() {
     json!({
       "aliases": {},
       "first": "foo",
+      "doc": null,
       "assignments": {
         "x": {
           "export": false,
@@ -341,6 +346,7 @@ fn duplicate_recipes() {
     ",
     json!({
       "first": "foo",
+      "doc": null,
       "aliases": {
         "f": {
           "attributes": [],
@@ -414,6 +420,7 @@ fn duplicate_variables() {
         }
       },
       "first": null,
+      "doc": null,
       "modules": {},
       "recipes": {},
       "settings": {
@@ -446,6 +453,7 @@ fn doc_comment() {
     json!({
       "aliases": {},
       "first": "foo",
+      "doc": null,
       "assignments": {},
       "modules": {},
       "recipes": {
@@ -494,6 +502,7 @@ fn empty_justfile() {
       "aliases": {},
       "assignments": {},
       "first": null,
+      "doc": null,
       "modules": {},
       "recipes": {},
       "settings": {
@@ -533,6 +542,7 @@ fn parameters() {
     json!({
       "aliases": {},
       "first": "a",
+      "doc": null,
       "assignments": {},
       "modules": {},
       "recipes": {
@@ -685,6 +695,7 @@ fn priors() {
       "aliases": {},
       "assignments": {},
       "first": "a",
+      "doc": null,
       "modules": {},
       "recipes": {
         "a": {
@@ -768,6 +779,7 @@ fn private() {
       "aliases": {},
       "assignments": {},
       "first": "_foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "_foo": {
@@ -815,6 +827,7 @@ fn quiet() {
       "aliases": {},
       "assignments": {},
       "first": "foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "foo": {
@@ -874,6 +887,7 @@ fn settings() {
       "aliases": {},
       "assignments": {},
       "first": "foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "foo": {
@@ -927,6 +941,7 @@ fn shebang() {
       "aliases": {},
       "assignments": {},
       "first": "foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "foo": {
@@ -974,6 +989,7 @@ fn simple() {
       "aliases": {},
       "assignments": {},
       "first": "foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "foo": {
@@ -1024,6 +1040,7 @@ fn attribute() {
       "aliases": {},
       "assignments": {},
       "first": "foo",
+      "doc": null,
       "modules": {},
       "recipes": {
         "foo": {
@@ -1068,6 +1085,7 @@ fn module() {
   Test::new()
     .justfile(
       "
+      # hello
       mod foo
     ",
     )
@@ -1082,11 +1100,13 @@ fn module() {
         "aliases": {},
         "assignments": {},
         "first": null,
+        "doc": null,
         "modules": {
           "foo": {
             "aliases": {},
             "assignments": {},
             "first": "bar",
+            "doc": "hello",
             "modules": {},
             "recipes": {
               "bar": {

tests/list.rs

@@ -353,3 +353,55 @@ fn nested_modules_are_properly_indented() {
     )
     .run();
 }
+
+#[test]
+fn module_doc_rendered() {
+  Test::new()
+    .write("foo.just", "")
+    .justfile(
+      "
+        # Module foo
+        mod foo
+      ",
+    )
+    .test_round_trip(false)
+    .args(["--unstable", "--list"])
+    .stdout(
+      "
+        Available recipes:
+            foo ... # Module foo
+      ",
+    )
+    .run();
+}
+
+#[test]
+fn module_doc_aligned() {
+  Test::new()
+    .write("foo.just", "")
+    .write("bar.just", "")
+    .justfile(
+      "
+        # Module foo
+        mod foo
+
+        # comment
+        mod very_long_name_for_module \"bar.just\" # comment
+
+        # will change your world
+        recipe:
+            @echo Hi
+      ",
+    )
+    .test_round_trip(false)
+    .args(["--unstable", "--list"])
+    .stdout(
+      "
+        Available recipes:
+            recipe                        # will change your world
+            foo ...                       # Module foo
+            very_long_name_for_module ... # comment
+      ",
+    )
+    .run();
+}