Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

--choose side pane not using correct just file #1638

Closed
simonmichael opened this issue Jul 7, 2023 · 4 comments · Fixed by #1759
Closed

--choose side pane not using correct just file #1638

simonmichael opened this issue Jul 7, 2023 · 4 comments · Fixed by #1759

Comments

@simonmichael
Copy link

simonmichael commented Jul 7, 2023

I have a non-default just file named eg justfoo, with a shebang line like

#!/usr/bin/env just -f

When I run ./justfoo --choose, the left pane lists justfoo's recipes, and they run correctly when selected. But the right pane seems to look for the default justfile instead, and shows error messages like "no justfile found" or "Justfile does not contain recipe ...". I expected it to show the recipes from justfoo.

Is this a bug ?

@Qeole
Copy link
Contributor

Qeole commented Dec 11, 2023

The default preview command for the chooser ('just --show {}') does not take the justfile into account, it runs another instance of just in a shell that only looks for the default justfile names.

It is possible to get the preview working by updating the chooser command:

$ export JUST_CHOOSER= "fzf --multi --preview 'just -f my_justfile --show {}'"
$ just -f my_justfile --choose

Locally, I "fixed" it with the following patch:

diff --git a/src/subcommand.rs b/src/subcommand.rs
index 99dacec47041..5d08b55d562d 100644
--- a/src/subcommand.rs
+++ b/src/subcommand.rs
@@ -213,10 +213,19 @@ impl Subcommand {
       return Err(Error::NoChoosableRecipes);
     }
 
+    let preview_file = match justfile.default {
+      Some(ref default) => format!(" -f {}'", default.name.path.display()),
+      None => "'".into(),
+    };
+
+    let mut default_chooser = OsString::from(config::CHOOSER_DEFAULT.strip_suffix("'")
+      .unwrap_or(config::CHOOSER_DEFAULT));
+    default_chooser.push(preview_file);
+
     let chooser = chooser
       .map(OsString::from)
       .or_else(|| env::var_os(config::CHOOSER_ENVIRONMENT_KEY))
-      .unwrap_or_else(|| OsString::from(config::CHOOSER_DEFAULT));
+      .unwrap_or_else(|| default_chooser);
 
     let result = justfile
       .settings

But trimming and modifying the default chooser string is not a clean solution, there's probably something better to do here.

@casey
Copy link
Owner

casey commented Dec 11, 2023

How about changing CHOOSER_DEFAULT from a constant to something like:

fn chooser_default(justfile: &Path) -> OsString {
  todo!()
}

Doing the interpolation will be a little funny, you can't interpolate a Path (Path::display should be avoided, since it'll munge non-unicode paths.).

@Qeole
Copy link
Contributor

Qeole commented Dec 12, 2023

How about changing CHOOSER_DEFAULT from a constant to something like: [...]

Looks better than my version :)

Doing the interpolation will be a little funny, you can't interpolate a Path (Path::display should be avoided, since it'll munge non-unicode paths.).

I suspected something like that, but I'm not familiar enough with Rust and paths (or OsStrings) to tell what's the best way to interpolate. Do you have any suggestion?

@casey
Copy link
Owner

casey commented Dec 12, 2023

I think maybe you'll have to create an OsString, and then append to it. Like so:

let mut chooser = OsString::new();
chooser.push("fzf --multi --preview 'just --justfile ");
chooser.push(path);
chooser.push(" --show {}'");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants