Repository: casey/just
Pull request #2113 (2024-05-30 07:01:19)

Allow listing recipes in submodules with `--list-submodules`
@crdx Do you prefer a nested view, like this:

```
Available recipes:
  foo
  bar

  abc:
    xyz
    wer

  hjk:
    yui
    asd
```

Or a flattened view, like this:

```
Available recipes:
  foo
  bar
  abc::xyz
  abc::wer
  hjk::yui
  hjk::asd
```


valscion: If I may add my preference? I'm in favor of the nested view as I'm not that comfortable showing `::` anywhere 😅 

The way we've used the "module-like" just scripts so far has relied on the separator being a space character and I'd like to avoid seeing the `::` in any output 😅 

I can see us using `--list-submodules` in some inner module, maybe already with the `just devices` case I pointed in https://github.com/casey/just/issues/929#issuecomment-2132780998  — but not at the top-level `just` case.

The `::` separator feels like some Rust style preference to me and not something I've seen in any other command line tool we use.
casey: The issue with spaces in a flattened view is that it wouldn't be possible to tell what was a submodule/recipe and what was an argument:

```
Available recipes:
  foo
  bar
  abc xyz
  abc wer
  hjk yui
  hjk asd
```

`abc` could be a recipe with an argument named `xyz`. The `::`-separated notation is also necessary in few places. I chose it because `::` is commonly used to separate module paths, e.g., in Rust, C++, and I think others.

The output of `--summary` uses spaces to separate recipe names, and completion scripts split on spaces, so in order to include recipes in submodules, they must be `::`-separated. Because of this, `::` must also be accepted on the command line, so that a recipe from the output of `--summary` can be passed to `just`.

When, eventually, you can reference variables and recipes in other modules, you'll have to specify module paths using `::`. This is just because the grammar would otherwise be ambiguous between submodule recipes and arguments:

```justfile
mod foo

bar: foo::baz
```

So the command-line interfaces will all accept `::` as an alternative to spaces, and when module paths eventually appear in `justfiles` themselves, they'll be space-separated. I do agree that spaces are nicer!

I'll probably add some sort of option to list submodules nested, as in this PR, and flattened, in another PR, using `::`.

As far as a depth limit, I'd be happy to add that in the future, feel free to make an issue if/when you want it!
valscion: Yeah to clarify my opinion — I don't have any objections of using `::` _inside_ Justfiles themselves. I do think it makes a lot of sense to use `::` inside Justfiles.

I'm mostly against showing that in the output of any `just` commands when ran on the command line.

I also do agree that the flattened view as in your example would be confusing and would not want that either.

It is indeed a tricky case! 

> The output of `--summary` uses spaces to separate recipe names, and completion scripts split on spaces, so in order to include recipes in submodules, they must be `::`-separated. Because of this, `::` must also be accepted on the command line, so that a recipe from the output of `--summary` can be passed to `just`.

I'm not sure I fully follow this train of thought, so let me try to understand what you mean with this example:

```
Available recipes:
  foo
  bar

  abc:
    xyz
    wer

  hjk:
    yui
    asd
```

I would image `just <TAB>` would show in autocompletion:

```
foo
bar
abc
hjk
```

And then I'd do `just abc <TAB>` and see:

```
xyz
wer
```

But it sounded like what I'd have instead is this when running `just <TAB>`?

```
foo
bar
abc::xyz
abc::wer
hjk::yui
hjk::asd
```

Do correct me if I'm wrong! I'm mostly worried that the tab autocompletions would get useless for us if all the submodule recipes would also be shown in there with the `::` separating them instead of needing "diving deeper" like we now have since https://github.com/casey/just/pull/2108
crdx: @casey The original nested output is my personal preference. I wouldn't use the flattened view.

Note: I don't use the concept of a default recipe, i.e. I don't have `just` on its own run anything. Instead, the first recipe is always:

```
[private]
help:
    just --list --unsorted
```

Because I prefer to see what's available and pick from the list rather than have a default. So, I can easily add a new flag `--nested` (or whatever) to this recipe everywhere.

@valscion I believe that the output of `--summary` is correct to show fully-qualified recipes. Technically a submodule recipe is still a recipe that can be run from the top-level context so completions should show it. And it needs to be `::`-separated because of completion word-splitting characters.

The current shell completions are generated by applying simple replacements to the clap-generated default completions:

https://github.com/casey/just/blob/f5bb82dea3edde07b5aca4303e2d73a300f45d5e/src/completions.rs

I wouldn't personally like to try to implement more intelligent submodule recipe completion handling like this. This is the point where hand-crafted completion scripts would be better.
valscion: > Technically a submodule recipe is still a recipe that can be run from the top-level context so completions should show it.

Yeah I do agree — but this "technically correct" vs. what I'm in favor of as "intuitive behavior" are different.

For us, it would make the autocompletion cumbersome and the first `tab` hit after entering `just <TAB>` would be useless as we'd see hundreds of potential autocompletions. At that point, we would not see the rest of the commands available at all.

For example, the `aws` CLI (which is huge!) also doesn't show "submodules" but instead lists the outermost modules first:

![image](https://github.com/casey/just/assets/482561/92292b29-4304-4c43-ad5d-cb89068bbe73)

Then I can tab to just the `aws s3` scripts:

![image](https://github.com/casey/just/assets/482561/aa1ea08b-0623-4c57-8372-7a262f6c0ce9)

This is how I'd expect "top level modules" to first work and then submodules showing only after I've explicitly tabbed to show the submodules.

Patch:
diff --git a/completions/just.bash b/completions/just.bash
index c5397794..e2513b01 100644
--- a/completions/just.bash
+++ b/completions/just.bash
@@ -30,7 +30,7 @@ _just() {
 
     case "${cmd}" in
         just)
-            opts="-n -f -q -u -v -d -c -e -l -s -E -g -h -V --check --chooser --color --command-color --yes --dry-run --dump-format --highlight --list-heading --list-prefix --no-aliases --no-deps --no-dotenv --no-highlight --justfile --quiet --set --shell --shell-arg --shell-command --clear-shell-args --unsorted --unstable --verbose --working-directory --changelog --choose --command --completions --dump --edit --evaluate --fmt --init --list --groups --man --show --summary --variables --dotenv-filename --dotenv-path --global-justfile --timestamp --timestamp-format --help --version [ARGUMENTS]..."
+            opts="-n -f -q -u -v -d -c -e -l -s -E -g -h -V --check --chooser --color --command-color --yes --dry-run --dump-format --highlight --list-heading --list-prefix --list-submodules --no-aliases --no-deps --no-dotenv --no-highlight --justfile --quiet --set --shell --shell-arg --shell-command --clear-shell-args --unsorted --unstable --verbose --working-directory --changelog --choose --command --completions --dump --edit --evaluate --fmt --init --list --groups --man --show --summary --variables --dotenv-filename --dotenv-path --global-justfile --timestamp --timestamp-format --help --version [ARGUMENTS]..."
                 if [[ ${cur} == -* ]] ; then
                     COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                     return 0
diff --git a/completions/just.elvish b/completions/just.elvish
index 18382eae..9655fe45 100644
--- a/completions/just.elvish
+++ b/completions/just.elvish
@@ -46,6 +46,7 @@ set edit:completion:arg-completer[just] = {|@words|
             cand -n 'Print what just would do without doing it'
             cand --dry-run 'Print what just would do without doing it'
             cand --highlight 'Highlight echoed recipe lines in bold'
+            cand --list-submodules 'List recipes in submodules'
             cand --no-aliases 'Don''t show aliases in list'
             cand --no-deps 'Don''t run recipe dependencies'
             cand --no-dotenv 'Don''t load `.env` file'
diff --git a/completions/just.fish b/completions/just.fish
index 7b5a1b45..4a49db2a 100644
--- a/completions/just.fish
+++ b/completions/just.fish
@@ -57,6 +57,7 @@ complete -c just -l check -d 'Run `--fmt` in \'check\' mode. Exits with 0 if jus
 complete -c just -l yes -d 'Automatically confirm all recipes.'
 complete -c just -s n -l dry-run -d 'Print what just would do without doing it'
 complete -c just -l highlight -d 'Highlight echoed recipe lines in bold'
+complete -c just -l list-submodules -d 'List recipes in submodules'
 complete -c just -l no-aliases -d 'Don\'t show aliases in list'
 complete -c just -l no-deps -d 'Don\'t run recipe dependencies'
 complete -c just -l no-dotenv -d 'Don\'t load `.env` file'
diff --git a/completions/just.powershell b/completions/just.powershell
index 5413bb1b..b39d367f 100644
--- a/completions/just.powershell
+++ b/completions/just.powershell
@@ -49,6 +49,7 @@ Register-ArgumentCompleter -Native -CommandName 'just' -ScriptBlock {
             [CompletionResult]::new('-n', 'n', [CompletionResultType]::ParameterName, 'Print what just would do without doing it')
             [CompletionResult]::new('--dry-run', 'dry-run', [CompletionResultType]::ParameterName, 'Print what just would do without doing it')
             [CompletionResult]::new('--highlight', 'highlight', [CompletionResultType]::ParameterName, 'Highlight echoed recipe lines in bold')
+            [CompletionResult]::new('--list-submodules', 'list-submodules', [CompletionResultType]::ParameterName, 'List recipes in submodules')
             [CompletionResult]::new('--no-aliases', 'no-aliases', [CompletionResultType]::ParameterName, 'Don''t show aliases in list')
             [CompletionResult]::new('--no-deps', 'no-deps', [CompletionResultType]::ParameterName, 'Don''t run recipe dependencies')
             [CompletionResult]::new('--no-dotenv', 'no-dotenv', [CompletionResultType]::ParameterName, 'Don''t load `.env` file')
diff --git a/completions/just.zsh b/completions/just.zsh
index 1c1fe92d..1c7dab01 100644
--- a/completions/just.zsh
+++ b/completions/just.zsh
@@ -44,6 +44,7 @@ _just() {
 '(-q --quiet)-n[Print what just would do without doing it]' \
 '(-q --quiet)--dry-run[Print what just would do without doing it]' \
 '--highlight[Highlight echoed recipe lines in bold]' \
+'--list-submodules[List recipes in submodules]' \
 '--no-aliases[Don'\''t show aliases in list]' \
 '--no-deps[Don'\''t run recipe dependencies]' \
 '--no-dotenv[Don'\''t load \`.env\` file]' \
diff --git a/src/config.rs b/src/config.rs
index cfe569cb..6c19c700 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -32,6 +32,7 @@ pub(crate) struct Config {
   pub(crate) invocation_directory: PathBuf,
   pub(crate) list_heading: String,
   pub(crate) list_prefix: String,
+  pub(crate) list_submodules: bool,
   pub(crate) load_dotenv: bool,
   pub(crate) no_aliases: bool,
   pub(crate) no_dependencies: bool,
@@ -97,11 +98,12 @@ mod arg {
   pub(crate) const DOTENV_PATH: &str = "DOTENV-PATH";
   pub(crate) const DRY_RUN: &str = "DRY-RUN";
   pub(crate) const DUMP_FORMAT: &str = "DUMP-FORMAT";
-  pub(crate) const GLOBAL_JUSTFILE: &str = "GLOBAL_JUSTFILE";
+  pub(crate) const GLOBAL_JUSTFILE: &str = "GLOBAL-JUSTFILE";
   pub(crate) const HIGHLIGHT: &str = "HIGHLIGHT";
   pub(crate) const JUSTFILE: &str = "JUSTFILE";
   pub(crate) const LIST_HEADING: &str = "LIST-HEADING";
   pub(crate) const LIST_PREFIX: &str = "LIST-PREFIX";
+  pub(crate) const LIST_SUBMODULES: &str = "LIST-SUBMODULES";
   pub(crate) const NO_ALIASES: &str = "NO-ALIASES";
   pub(crate) const NO_DEPS: &str = "NO-DEPS";
   pub(crate) const NO_DOTENV: &str = "NO-DOTENV";
@@ -112,7 +114,7 @@ mod arg {
   pub(crate) const SHELL_ARG: &str = "SHELL-ARG";
   pub(crate) const SHELL_COMMAND: &str = "SHELL-COMMAND";
   pub(crate) const TIMESTAMP: &str = "TIMESTAMP";
-  pub(crate) const TIMESTAMP_FORMAT: &str = "TIMESTAMP_FORMAT";
+  pub(crate) const TIMESTAMP_FORMAT: &str = "TIMESTAMP-FORMAT";
   pub(crate) const UNSORTED: &str = "UNSORTED";
   pub(crate) const UNSTABLE: &str = "UNSTABLE";
   pub(crate) const VERBOSE: &str = "VERBOSE";
@@ -236,6 +238,13 @@ impl Config {
           .value_name("TEXT")
           .action(ArgAction::Set),
       )
+      .arg(
+        Arg::new(arg::LIST_SUBMODULES)
+          .long("list-submodules")
+          .help("List recipes in submodules")
+          .action(ArgAction::SetTrue)
+          .env("JUST_LIST_SUBMODULES"),
+      )
       .arg(
         Arg::new(arg::NO_ALIASES)
           .long("no-aliases")
@@ -754,6 +763,7 @@ impl Config {
       list_prefix: matches
         .get_one::<String>(arg::LIST_PREFIX)
         .map_or_else(|| "    ".into(), Into::into),
+      list_submodules: matches.get_flag(arg::LIST_SUBMODULES),
       load_dotenv: !matches.get_flag(arg::NO_DOTENV),
       no_aliases: matches.get_flag(arg::NO_ALIASES),
       no_dependencies: matches.get_flag(arg::NO_DEPS),
diff --git a/src/subcommand.rs b/src/subcommand.rs
index 09aacbee..77658ac2 100644
--- a/src/subcommand.rs
+++ b/src/subcommand.rs
@@ -488,6 +488,12 @@ impl Subcommand {
         .ok_or_else(|| Error::UnknownSubmodule { path: path.clone() })?;
     }
 
+    Self::list_module(config, module, 0);
+
+    Ok(())
+  }
+
+  fn list_module(config: &Config, module: &Justfile, depth: usize) {
     let aliases = if config.no_aliases {
       BTreeMap::new()
     } else {
@@ -532,7 +538,11 @@ impl Subcommand {
       .max()
       .unwrap_or(0);
 
-    print!("{}", config.list_heading);
+    let list_prefix = config.list_prefix.repeat(depth + 1);
+
+    if depth == 0 {
+      print!("{}", config.list_heading);
+    }
 
     let groups = {
       let mut groups = BTreeMap::<Option<String>, Vec<&Recipe>>::new();
@@ -557,7 +567,7 @@ impl Subcommand {
       let no_groups = groups.contains_key(&None) && groups.len() == 1;
 
       if !no_groups {
-        print!("{}", config.list_prefix);
+        print!("{list_prefix}");
         if let Some(group_name) = group {
           println!("[{group_name}]");
         } else {
@@ -580,8 +590,7 @@ impl Subcommand {
             if doc.lines().count() > 1 {
               for line in doc.lines() {
                 println!(
-                  "{}{} {}",
-                  config.list_prefix,
+                  "{list_prefix}{} {}",
                   config.color.stdout().doc().paint("#"),
                   config.color.stdout().doc().paint(line),
                 );
@@ -590,8 +599,7 @@ impl Subcommand {
           }
 
           print!(
-            "{}{}",
-            config.list_prefix,
+            "{list_prefix}{}",
             RecipeSignature { name, recipe }.color_display(config.color.stdout())
           );
 
@@ -611,11 +619,21 @@ impl Subcommand {
       }
     }
 
-    for submodule in module.modules(config) {
-      println!("{}{} ...", config.list_prefix, submodule.name(),);
-    }
+    if config.list_submodules {
+      for (i, submodule) in module.modules(config).into_iter().enumerate() {
+        if i + groups.len() > 0 {
+          println!();
+        }
 
-    Ok(())
+        println!("{list_prefix}{}:", submodule.name());
+
+        Self::list_module(config, submodule, depth + 1);
+      }
+    } else {
+      for submodule in module.modules(config) {
+        println!("{list_prefix}{} ...", submodule.name(),);
+      }
+    }
   }
 
   fn show<'src>(


Test patch:
diff --git a/tests/list.rs b/tests/list.rs
index e8002b13..7c8a26bd 100644
--- a/tests/list.rs
+++ b/tests/list.rs
@@ -223,3 +223,133 @@ fn list_unknown_submodule() {
     .status(1)
     .run();
 }
+
+#[test]
+fn list_with_groups_in_modules() {
+  Test::new()
+    .justfile(
+      "
+        [group('FOO')]
+        foo:
+
+        mod bar
+      ",
+    )
+    .write("bar.just", "[group('BAZ')]\nbaz:")
+    .test_round_trip(false)
+    .args(["--unstable", "--list", "--list-submodules"])
+    .stdout(
+      "
+        Available recipes:
+            [FOO]
+            foo
+
+            bar:
+                [BAZ]
+                baz
+      ",
+    )
+    .run();
+}
+
+#[test]
+fn list_displays_recipes_in_submodules() {
+  Test::new()
+    .write("foo.just", "bar:\n @echo FOO")
+    .justfile(
+      "
+        mod foo
+      ",
+    )
+    .test_round_trip(false)
+    .args(["--unstable", "--list", "--list-submodules"])
+    .stdout(
+      "
+      Available recipes:
+          foo:
+              bar
+    ",
+    )
+    .run();
+}
+
+#[test]
+fn modules_are_space_separated_in_output() {
+  Test::new()
+    .write("foo.just", "foo:")
+    .write("bar.just", "bar:")
+    .justfile(
+      "
+        mod foo
+
+        mod bar
+      ",
+    )
+    .test_round_trip(false)
+    .args(["--unstable", "--list", "--list-submodules"])
+    .stdout(
+      "
+      Available recipes:
+          bar:
+              bar
+
+          foo:
+              foo
+    ",
+    )
+    .run();
+}
+
+#[test]
+fn module_recipe_list_alignment_ignores_private_recipes() {
+  Test::new()
+    .write(
+      "foo.just",
+      "
+# foos
+foo:
+ @echo FOO
+
+[private]
+barbarbar:
+ @echo BAR
+
+@_bazbazbaz:
+ @echo BAZ
+      ",
+    )
+    .justfile("mod foo")
+    .test_round_trip(false)
+    .args(["--unstable", "--list", "--list-submodules"])
+    .stdout(
+      "
+        Available recipes:
+            foo:
+                foo # foos
+      ",
+    )
+    .run();
+}
+
+#[test]
+fn nested_modules_are_properly_indented() {
+  Test::new()
+    .write("foo.just", "mod bar")
+    .write("bar.just", "baz:\n @echo FOO")
+    .justfile(
+      "
+        mod foo
+      ",
+    )
+    .test_round_trip(false)
+    .args(["--unstable", "--list", "--list-submodules"])
+    .stdout(
+      "
+      Available recipes:
+          foo:
+              bar:
+                  baz
+    ",
+    )
+    .run();
+}
diff --git a/tests/test.rs b/tests/test.rs
index 16b3b0e7..c57a4c45 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -258,7 +258,7 @@ impl Test {
       }
     }
 
-    if !compare("status", output.status.code().unwrap(), self.status)
+    if !compare("status", output.status.code(), Some(self.status))
       | (self.stdout_regex.is_none() && !compare("stdout", output_stdout, &stdout))
       | (self.stderr_regex.is_none() && !compare("stderr", output_stderr, &stderr))
     {