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

Make the else branch of a conditional optional #1657

Closed
xavdid opened this issue Aug 4, 2023 · 9 comments · Fixed by #1845
Closed

Make the else branch of a conditional optional #1657

xavdid opened this issue Aug 4, 2023 · 9 comments · Fixed by #1845

Comments

@xavdid
Copy link

xavdid commented Aug 4, 2023

I have the following recipe:

@bump package:
    {{ if path_exists("Formula" / package + ".rb") == "false" { error("no formula for package " + package) } else { "" } }}

Which works great! But, it gives an error if I don't include a dud else block.

error: Expected keyword `else` but found `'}}'`
  |
7 |     {{ if path_exists("Formula" / package + ".rb") == "false" { error("no formula for package " + package) } }}

It would be nice if it were optional

@casey
Copy link
Owner

casey commented Oct 9, 2023

All just expressions return a value, so we could only make this work if the if branch diverges, e.g., does not return. So it could work for the error case, but couldn't work in general. One possibility is to add an error_if(CONDITION, MESSAGE) function.

@Julian
Copy link

Julian commented Oct 20, 2023

Why is that? I too was tempted into simply using else { "" } to workaround this (before finding this issue). Surely even if the if doesn't diverge it could simply default to an else branch with some default value like "" as its "return value", which is highly likely to be ignored?

@mike-lloyd03
Copy link

I'm all for an error_if function. I'm currently using else {""} as a workaround but it's not very clean. I'm migrating most of my stuff over from make and enjoy the cleaner and simpler syntax of just so this solution seems like a good fit.

@casey
Copy link
Owner

casey commented Nov 17, 2023

I'm still not totally sold, but if we did implement this, how about a function called ensure(CONDITION, MESSAGE), which takes a boolean CONDITION, and errors out with MESSAGE if it's false?

One reason I'm not totally sold is that it seems like this could be done in shell.

For example, for the use-case in the issue, you could do:

foo:
  test -e {{("Formula" / package + ".rb"}} || exit "no formula for package {{package}}"

@Julian
Copy link

Julian commented Nov 17, 2023

One reason I'm not totally sold is that it seems like this could be done in shell.

Personally I want to write literally the minimum amount of shell ever possible. I realize part of that is counter to using just at all, but certainly I try to minimize any shell I write regardless.

Here are two places (the other a few lines down, where the condition is inverted) where I basically use the workaround discussed here -- I do have to say I personally find it surprising to be missing but I'm simply someone who's used / evaluated just once (to write that justfile) -- and certainly ensure(...) would be better than nothing so as a user I'd certainly prefer that if that's what you'd be comfortable with!

@xavdid
Copy link
Author

xavdid commented Nov 17, 2023

+1 - I'm using just's functions because I want to avoid finicky shell scripts.

I think ensure would work here! sort of like an assert in other languages

@casey
Copy link
Owner

casey commented Nov 17, 2023

assert is probably a better name, since it's more familiar. Yah, agree about shell being tricky. And even if the just syntax is slightly worse, being statically typed is a big advantage.

Okay, I think I'm sold. If anyone wants to take a crack at this, it would make a good first PR!

@xavdid
Copy link
Author

xavdid commented Nov 18, 2023

Nice, appreciate it!

I won't have time to look at this for a few months (and I have to brush up on my Rust). But if it's still available then, I'll take a crack at it!

@xavdid
Copy link
Author

xavdid commented May 15, 2024

Awesome, thank you!

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

Successfully merging a pull request may close this issue.

4 participants