Coalesce-like string function

[faultymajority]

In a Justfile I'm building, I have use for "optional arguments". It would be helpful to have something like

target a *b:
  echo {{ coalesce(b, a) }}

where the return value of coalesce is b is passed (i.e., its value has length > 0), and a otherwise. In that way, b would be an optional argument, with a as default.

Would a PR for this be welcome? The function name would probably need bike shedding, as SQL coalesce operates based on the first argument being NULL, not "empty".

It would (for my use case) be even nicer if there were syntax

target a *b=a:
  echo {{ b }}

which currently fails with "error: Variable a not defined". Could this ever work? As the pieces seem there:

x := "abc"
target a *b=x:
  echo {{ b }}

UDFs in general Generally - for "user-defined functions" I'm currently using the following pattern

coalesce a *b:
  @echo {{ a }}
target a *b:
  echo $(just coalesce {{ b }} {{ a }})

Is this the intended way to have UDFs?

[laniakea64]

It's already possible:

target a *b:
  echo {{ if b == '' { a } else { b } }}

It would (for my use case) be even nicer if there were syntax

target a *b=a:
  echo {{ b }}

which currently fails with "error: Variable a not defined". Could this ever work?

Seems it does work?

a := ''
target a *b=a:
  echo {{ b }}

$ just target 123
echo 123
123
$ just target 123 456
echo 456
456

[faultymajority]

It's already possible:

target a *b:
  echo {{ if b == '' { a } else { b } }}

Good to know, although this is a bit repetitive.

Seems it does work?

a := ''
target a *b=a:
  echo {{ b }}

Oh this is interesting - it would be a bit nicer not to have to declare a in the global namespace, but I wouldn't have guessed this, thanks!

[casey]

Oh this is interesting - it would be a bit nicer not to have to declare a in the global namespace, but I wouldn't have guessed this, thanks!

lmao I have no idea why you have to declare a, this is definitely not intended behavior.

[casey]

Opened an issue to figure this out #2381.

[casey]

As of #2382, you can now use earlier recipe parameters in later recipe parameter default values, so this will now work:

target a *b=a:
  echo {{ b }}

This was so close to working, the fix was very small, just two trivial changes in one place, and there was even a test which checked that it was an error. I'm really hoping that past me didn't have some good reason for not allowing this.

The only reason I can think to not allow this is if you want to reference an outer variable but can't because you're shadowing it with a prior paramter. However, we already had shadowing, as @laniakea64 discovered (how did you figure this out btw?), which we couldn't remove without a backwards-incompatible change, so I can't really think of a reason not to enable it.

[casey]

Is this the intended way to have UDFs?

Not really, but only because there isn't any way to create user-defined functions. #1059 tracks adding user-defined functions, although there's been no movement on it for a while since it's hard. I started a PR to add it but then got demoralized when I realized it would be tricky 😂

Should I leave this issue open or does allowing parameters to reference each other work for your use case? A coalesce function wouldn't be too hard to add, although it seems like it might have limited applicability.

[laniakea64]

we already had shadowing, as @laniakea64 discovered (how did you figure this out btw?)

By being about to answer "this would break backwards compatibility with using the global variable a in that position" to this issue, was going to include an example to demonstrate what was happening, but running the example showed that answer would've been completely wrong :sweat_smile:

[casey]

Very nice catch!

I just bisected and figured out what happened. Originally, having a parameter shadow a global variable was forbidden. Then it was allowed, but uses of that parameter name would always refer to the global variable, even if a previous parameter had the same name.

Then, in #2138, which refactored the evaluator, this behavior changed, and uses of that variable name in default values would refer to the previous parameter.

This was due to a somewhat subtle change, from this:

let mut scope = scope.child();
…
scope.bind(parameter.export, parameter.name, value);

To this:

let mut evaluator = Self::new(context, context.scope);
…
evaluator
  .scope
  .bind(parameter.export, parameter.name, value);

So now, parameters are defined directly in the evaluator scope, instead of a separate scope, where they can now affect the evaluation of following parameters.

This change was not actually made by the author of the PR, @neunenak, who is wholly innocent, but by me, in some random commits I piled on afterwards.

I looked at the rest of the PR, and this is the only place with such a change, so there probably weren't any other unexpected changes to evaluation scope.

This was an unintentional backwards-incompatible change, with the first version with the new behavior being 1.29, published on 6/13/2024, or a little under three months ago.

So, this is a somewhat interesting scenario in that we have a fairly recent, unintentional backwards incompatible change.

just's policy is to not make backwards incompatible changes on purpose. But if they're made accidentally, it shouldn't be the case, necessarily, that they're reverted. If a change is neutral or beneficial, and more disruption would be caused by reverting it, we should probably let it be.

This change seems beneficial to me. With it, if you want to access a global variable in a default value, you can simply use a different name for the previous parameter that shadows it. However without it, there is no way to access the value of a previous parameter.

It would be great to know how many users are on previous versions, since if there were large numbers of users on 1.28 or earlier, then it is conceivable that, if the change were disruptive enough, reverting it would be a net benefit.

Looking at the to packaging status tables:

https://repology.org/badge/vertical-allrepos/rust:just.svg

https://repology.org/badge/vertical-allrepos/just.svg

The largest distros that are on 1.28 and earlier are nixpkgs stable, Alpine, Guix, and Ubuntu 24.04 LTS. All the rest are on 1.29 or later.

So I think that since this is mildly beneficial, most users are on newer versions, it would probably be more disruptive to revert than not, and we haven't actually gotten any bug reports about it, we should probably just let it ride.