template: add `env()` function by chawyehsu · Pull Request #9084 · jj-vcs/jj

chawyehsu · 2026-03-11T15:47:12Z

WIP. Relates to #5571

Checklist

If applicable:

I have updated CHANGELOG.md
I have updated the documentation (README.md, docs/, demos/)
I have updated the config schema (cli/src/config-schema.json)
I have added/updated tests to cover my changes
I fully understand the code that I am submitting (what it does,
how it works, how it's organized), including any code drafted by an LLM.
For any prose generated by an LLM, I have proof-read and copy-edited with
an eye towards deleting anything that is irrelevant, clarifying anything
that is confusing, and adding details that are relevant. This includes,
for example, commit descriptions, PR descriptions, and code comments.

chawyehsu · 2026-03-11T16:03:30Z

cli/src/template_builder.rs

+        let var_name = template_parser::catch_aliases(diagnostics, var_name_node, |_diagnostics, node| {
+            template_parser::expect_string_literal(node)
+        })?;
+        let out_property = std::env::var(&var_name).unwrap_or_default();


Concerning unwrap_or_default():

One thing I'm not yet certain about is whether to support “checking if an environment variable is null-defined” i.e., environment variable defined like export JJ_TEMPLATE_ENV_X.

If needed, then optional strings should to be introduced into the template engine to achieve results like if(env("JJ_TEMPLATE_ENV_X"),...) that enter the truthy branch. This would be roughly similar to ConfigValueOpt and IntegerOpt.

Seems like returning an Option<String> would be consistent with the existing config() function, so that could be a good reasoning for it. Also comparing Option<T> with a T will always return false, so the basic usage of if(env("MY_VAR") == "test", ...) would work as expected. However Option is documented as Template: maybe, and I'm not sure if that means that an Option<String> will silently be turned into "" if it's unset and you try to put it somewhere where a Template is expected (e.g., ++).

Option<String> (and Option<bool>) can't be added because String supports implicit boolean cast. If we're going to add Option<String> type, we should make if("") evaluates to true.

Hm, that's a good point. The docs say:

An option can be implicitly converted to Boolean denoting whether the contained value is set.

But I guess that only applies to the currently-supported Option<T> types, which doesn't include potentially-confusing cases like Some(false) and Some("").

I guess another alternative could be a new env_is_set(name: String) -> Boolean function in parallel to this one?

If we really need env_is_set(), I would introduce breaking change on if(""). There are other functions that make sense to return Option<String>.

I think if("") evaluating to true is also a breaking change right? I think a lot of people (myself included) depend on functionality such as if(description) to evaluate to false for an empty string.

What about instead, adding an Option.is_set() -> Boolean method, but normal Boolean conversion falls back to the underlying T? So Some("") would return false, but Some("").is_set() would return true (and None and None.is_set() would both return false). This is getting out of scope for the current PR, though.

because String supports implicit boolean cast. ...make if("") evaluates to true.

This is the main reason I'm unsure whether to introduce Option<String>.

Initially, I personally felt that env() shouldn't support checking null-defined environment variables, believing the need for this wasn't as high as the need to read values, and this is the current implementation. While it's intuitive that if("") evaluates to false, it semantically can indeed evaluate to true.

env_is_set(name: String) -> Boolean function in parallel

I considered a similar pair function solution env_exist(), but I don't particularly like this slightly redundant interface form, even though it's simple and straightforward.

I think whether to introduce Option<String> and the semantic-breaking change regarding if("") really depends on how much the other part in template engine needs Option<String>. And I can't say I understand this better than you.

If the current need for Option<String> isn't as significant as imagine, perhaps having a dedicated Environment return type for env() could be also considered, but it would still need a .is_set() method to handle similar use cases. This is somewhat similar to ConfigValue and ConfigValue::as_boolean.

This is getting out of scope for the current PR, though.

However, this is indeed relevant to this PR. Unless a better solution emerges, if Option<String> is considered, then this PR will be stacked on that.

then this PR will be stacked on that.

Sorry yeah I meant that a larger decision around Option<String> / the other things discussed in this thread might need to be another PR, which this one depends on since it directly affects how the env() function behaves.

I personally felt that env() shouldn't support checking null-defined environment variables, believing the need for this wasn't as high

I don't think it's common to set a boolean environment variable with "", so I think it's okay to use "" as default value. I think we can revisit this topic when someone complains about it.

Option.is_set() -> Boolean

Option can't have method table because opt.f() is translated to opt?.f() internally.

yuja · 2026-03-12T08:14:59Z

cli/src/template_builder.rs

+            template_parser::catch_aliases(diagnostics, var_name_node, |_diagnostics, node| {
+                template_parser::expect_string_literal(node)
+            })?;
+        let out_property = std::env::var(var_name).unwrap_or_default();


nit: Can we pass HashMap<String, String> as evaluation context instead? I think it's better to not access std::env::var() in templater. For example, the current working directory is specified by path_converter: &'repo RepoPathUiConverter

Perhaps, we can reuse ConfigEnv::environment.

This is fairly reasonable. I think reusing ConfigEnv's environment would be a good idea for deterministic evaluation, but as of now I definitely need a deeper understanding of jj's codebase to understand how to pass it into the templater's context and use it, considering this is my first contribution. ;p

yuja · 2026-03-12T08:18:07Z

cli/src/template_builder.rs

+        let var_name = template_parser::catch_aliases(diagnostics, var_name_node, |_diagnostics, node| {
+            template_parser::expect_string_literal(node)
+        })?;
+        let out_property = std::env::var(&var_name).unwrap_or_default();


Option<String> (and Option<bool>) can't be added because String supports implicit boolean cast. If we're going to add Option<String> type, we should make if("") evaluates to true.

Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>

to reuse environment hashmap passed from `ConfigEnv::environment` instead of `std::env::var`. Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>

chawyehsu commented Mar 11, 2026

View reviewed changes

chawyehsu force-pushed the feature/template-env-function branch from 5665536 to 19f37a8 Compare March 11, 2026 16:06

yuja reviewed Mar 12, 2026

View reviewed changes

chawyehsu added 2 commits March 14, 2026 13:07

template: add env() function

406c41c

Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>

templater: provide environment variables accessing in template languages

45bbcbd

to reuse environment hashmap passed from `ConfigEnv::environment` instead of `std::env::var`. Signed-off-by: Chawye Hsu <su+git@chawyehsu.com>

chawyehsu force-pushed the feature/template-env-function branch from bbee312 to 45bbcbd Compare March 14, 2026 05:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

template: add `env()` function#9084

template: add `env()` function#9084
chawyehsu wants to merge 2 commits intojj-vcs:mainfrom
chawyehsu:feature/template-env-function

chawyehsu commented Mar 11, 2026

Uh oh!

chawyehsu Mar 11, 2026

Uh oh!

josephlou5 Mar 12, 2026

Uh oh!

yuja Mar 12, 2026

Uh oh!

josephlou5 Mar 12, 2026

Uh oh!

yuja Mar 12, 2026

Uh oh!

josephlou5 Mar 12, 2026

Uh oh!

chawyehsu Mar 12, 2026

Uh oh!

josephlou5 Mar 12, 2026

Uh oh!

yuja Mar 13, 2026

Uh oh!

yuja Mar 12, 2026

Uh oh!

chawyehsu Mar 12, 2026 •

edited

Loading

Uh oh!

yuja Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

chawyehsu commented Mar 11, 2026

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chawyehsu Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chawyehsu Mar 12, 2026 •

edited

Loading