casey / just

🤖 Just a command runner
https://just.systems
Creative Commons Zero v1.0 Universal
20.36k stars 451 forks source link

[feature] User-defined functions #1059

Open rileyshahar opened 2 years ago

rileyshahar commented 2 years ago

Awesome tool, I'm mostly very happy with it, and this is a relatively minor QOL issue issue with two workarounds I mention below.

My use-case is something like:

build_dir := "build"
file_name := "out"

default arg="": (html arg) (pdf arg)

html arg="":
    pandoc input.md -o "{{build_dir}}/{{arg}}/{{file_name}}.html"

pdf arg="":
    pandoc input.md -o "{{build_dir}}/{{arg}}/{{file_name}}.pdf"

I would like a way to deduplicate the specific string formatting {{build_dir}}/{{arg}}/{{file_name}} so that my uses of it don't become out of sync. My current solution is to use arg as a just variable instead of an argument, i.e. arg := "" and file_path := build_dir + "/" + arg + "/" + file_name at the top of the Justfile. This is imperfect because it requires passing the argument to the CLI as just pdf arg=foo instead of just pdf foo, and also it puts into global scope something that shouldn't necessarily be there.

Another workaround is something like this:

[...]

default arg="": (_default (build_dir + "/" + arg + "/" + file_name))
_default arg: (html arg) (pdf arg)

html arg="":
    pandoc input.md -o "{{arg}}.html"

pdf arg="":
    pandoc input.md -o "{{arg}}.pdf"

But this is inelegant and doesn't allow the html and pdf targets to be run directly.

Is there a better way to accomplish this currently? If not, is it possible to add some syntax for declaring functions on strings? I'm imagining something like (this is obviously just a first draft at a syntax):

buildpath(s) := build_dir + "/" + s + "/" + file_name

pdf arg="":
    pandoc input.md -o "{{buildpath(arg)}}.pdf"

I haven't looked super closely into the code base so I'm not sure how easy this would be. I'd be happy to look into writing a PR for this if it's desirable and feasible. Regardless, thanks for a great tool.

casey commented 2 years ago

This is definitely desirable!

I like the syntax you proposed, i.e.:

buildpath(s) := build_dir + "/" + s + "/" + file_name

One thing that strikes me is that recipes are already similar to functions, so another option would be to allow calling recipes in expressions, which would return their standard output:

build_dir := "build"
file_name := "out"

default arg="": (html arg) (pdf arg)

path arg:
  echo {{build_dir}}/{{arg}}/{{file_name}}

html arg="":
    pandoc input.md -o "{{(path arg)}}.html"

pdf arg="":
    pandoc input.md -o "{{(path arg)}}.pdf"

This might be easier, since a lot of the infrastructure (error messages, etc) is already there.

rileyshahar commented 2 years ago

Cool! Here are some initial thoughts on steps for implementation:

  1. Add a new item, closure, to the grammar:
    
    item          : recipe
                 | [...]
                 | closure

closure : NAME '(' sequence? ')' ':=' expression eol


2. Implement an AST item closure, defined as
```rust
pub struct Closure<'src> {
    parameter: Name<'src>, // something a little more complex is actually needed to model multivariate functions
    rule: Expression<'src>,
}

pub type NamedClosure<'src> = Binding<'src, Closure>

You could reuse Assignment here, but that requires Closure to be added to the Expression enum, which I think is not desirable, since you don't want a closure to be a subterm of an expression.

  1. Parse closures, presumably by adding another case to the conditional here: https://github.com/casey/just/blob/ef3629fae333c7b9d1318421ce381543a9a6c51f/src/parser.rs#L340-L345

  2. Evaluate closures to Functions via capturing the context. Maybe we only need a reference here or maybe we need to clone, I need to look more into it to know for sure - what do you think?

  3. Add a variant of Thunk which doesn't need the function until evaluation time.

Obviously this is a pretty big change so I expect to run into other issues during implementation, but how do you feel about this general outline? I'm definitely open to other strategies.

(Of course error handling, tests, etc. are also necessary, this just describes the happy path.)

casey commented 2 years ago

Obviously this is a pretty big change so I expect to run into other issues during implementation, but how do you feel about this general outline? I'm definitely open to other strategies.

The outline sounds great!

One general suggestion I'd make, since this is a big feature, is that anything that can reasonably be left for a follow-up PR should be. Better to get something bare-bones in than to make the PR harder to land. So tests and documentation should definitely be done as part of the PR, but if there are additional nice-to-have features that can be left out, they should be if they make the PR more complicated.

As for tests, I have a lot of unit tests, but I don't think it's important to have full unit test coverage. I like to have full test coverage via integration tests, and then I mostly use unit tests when something is tricky to write or debug, in which case I write unit tests expressing the correct behavior. So don't worry about unit tests unless you find them useful while developing.

If they're recursive, custom functions will probably push just into Turing-completeness, see #792.

yuri-potatoq commented 10 months ago

Hi everybody, I'm really excited about working on this issue. I'm not a pro with parser stuff, but i'm studying this code to figure out how hard it can be.

If there is someone who wants to pair with me, i would really appreciate it. I would really love to want these custom functions feature 🚀

Splines commented 2 months ago

I came here because I misunderstood this sentence:

The [private] attribute may also be used to hide recipes or aliases without needing to change the name. [...] This is useful for helper recipes which are only meant to be used as dependencies of other recipes.

I thought "dependency" meant that I could call another recipe and capture its output as described in this issue 😅 But unfortunately that's not possible (yet). "Dependency" is meant in the sense of this phrase: "Dependencies run before recipes that depend on them".

rougsig commented 1 month ago

As a workaround:

// justfile.shell.zsh
get_version () {
  echo $(cat $1/package.json | jq -r '.version')
}

eval $1
// justfile
set shell := ["zsh", "./justfile.shell.zsh"]

sample:
  echo $(get_version app)