#340 Allow justfile without workdir

[smonami]

#340 Making --working-directory optional for --justfile and --edit.

[casey]

Thank you for the patch! I have a few suggestions for how to avoid some unwraps.

I think this needs a test that ensures that the correct working directory is chosen. Would you like to take a stab at writing one? You can check out the existing tests in the tests directory for ideas. I'm also happy to write it myself.

[casey]

It looks like this change isn't needed.

[smonami]

Corrected extra space.

[casey]

I think that since the behavior of just in setting the working directory is the same as normal when using this flag, we can make the comment shorter. Just Use <JUSTFILE> as justfile. is fine.

[smonami]

Done.

[casey]

Let's change working_directory_option to a Option<&Path>, so that we're working with paths from here on out, and don't have to convert to a string. You can do .map(Path::new).

[smonami]

Done.

[casey]

Let's change this to an if let, so we can avoid the unwraps:

if let (Some(justfile), None) = (justfile_option, working_directory_option) {

[smonami]

Done.

[casey]

If parent() returns none, we should exit with a message, like die!("Could not find parent directory of justfile at {}", justfile)

[smonami]

Added none check. But there seems to be a known issue(rust-lang/rust#36861) with parent returning "" in certain cases. Please suggest if it can be handled in a better way.

[casey]

Thanks for the changes! Let's resolve the justfile to an absolute path, and then using the parent of that.

[casey]

That should fix the issue with parent returning ""

[smonami]

Disclaimer: I am very new to Rust :-), and although I understand reference concept, not able to find a 'Rust way' to deal with following situation and so having trouble with the code:

if let (Some(justfile), None) = (justfile_option, working_directory_option) {
     let justfile_path = Path::new(justfile);
    if Path::new(justfile).is_absolute() {
        working_directory_option = Path::new(justfile).parent().unwrap().to_str();
    } else {
      let justfile_path_canonical = justfile_path.canonicalize();
      if justfile_path.canonicalize().is_ok() {
// @casey following line errors out with borrowed value does not live long enough. Any suggestion?
        working_directory_option = justfile_path_canonical.ok().unwrap().to_str(); 
      } else {
        die!("Could not find parent directory of justfile at {}.", justfile);
      }
    }
    //println!("working_directory_option:{:?}", working_directory_option);
  }

[casey]

No worries at all, we were all new to Rust once :)

Can you push the code (even if it's broken) to the PR branch so I can try it out?

[smonami]

Done. It's broken.

[casey]

I changed it to turn justfile_option and working_directory_option in a Path and Pathbuf respectively. Using path types simplifies the code because now conversions aren't required to and from &str. working_directory_option is a PathBuf so that it can be modified if need be, and this also solves the lifetime problem. This still needs a test, but I'm happy to write one if you'd rather not.

[casey]

This will need a full integration test, i.e. a test that makes sure that the working directory is correct when --working-directory isn't passed.

[casey]

Just added a test and landed this in #392. Thank you for contributing!