fizyr / subst

Apache License 2.0
10 stars 4 forks source link

feat!: Add partial and permissive substitutions and recursive braced variables substitution #18

Open D-Brox opened 2 months ago

D-Brox commented 2 months ago

Allow three modes of substitution:

This PR also fixes recursive substitution to allow braced variables, so ${va1:${var2}} is now also allowed. Cherry-picked to main in 476997bd40cd4ed2339b01a344578b4513c8a513

This is a breaking change, as the function signatures have changed.

D-Brox commented 2 months ago

I think I may need to add more to the documentation

de-vri-es commented 4 weeks ago

Thanks for the PR, and sorry for the delay!

I've cherry-picked the fix to main and released it as v0.3.1.

For the mode changes: I do like the idea of allowing the user to choose what to do on missing variables.

But there is a bit of murky territory: If you have "${a:${b:$c}}", and none of the variables are defined, what do we expect Partial to do here? As a user, I think I would expect the string to be untouched, but in practise this will probably result in "$c"? (Which is much easier in the implementation and arguably still correct, possibly even desirable.)

I also don't want to break backwards compatibility for adding this. To avoid that, I see two ways forward:

1) Extend the VariableMap trait to allow it to make this choice. Then users can wrap their variable map in a wrapper type to change the way undefined variables are handled.

2) Implement a Template struct that holds a parsed template, and where the user can set this mode before performing substitution. Added benefit is that it allows for more efficient expansion of the same string twice.

The existing functions would remain as short-hand for instantiating a template and performing expansion.

But I have some pretty specific requirements in mind for this struct, so please don't start on it without discussing first :)
D-Brox commented 3 weeks ago

I see, there really is ambiguity in the Partial mode. The current implementation would indeed replace ${a:${b:$c}} with $c. Maybe we could add a Lazy mode that leaves it untouched, making the distinction more clear.

About your suggestions, 1. should be easier and faster to implement, but I personally like the idea in 2.

de-vri-es commented 3 weeks ago

I added re-usable templates in #21. Maybe you can also look at that PR to evaluate if there is anything you think should be different to add multiple expansion modes in the future.

D-Brox commented 2 weeks ago

I'll look at it over the weekend