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

syntax: simplify unset-or-null with null default to just unset #849

Merged
merged 1 commit into from
May 23, 2022

Conversation

scop
Copy link
Contributor

@scop scop commented Apr 14, 2022

If the default for an unset-or-null parameter expansion is null, the or-null part of the expansion is redundant and can be removed. The result is null on match either way.

If the default for an unset-or-null parameter expansion is null, the
or-null part of the expansion is redundant and can be removed. The
result is null on match either way.
@scop scop force-pushed the unset-or-null-null-default branch from 2755b94 to 5635541 Compare April 15, 2022 05:44
@scop
Copy link
Contributor Author

scop commented Apr 15, 2022

Docker build failure/timeout appears unrelated.

@mvdan
Copy link
Owner

mvdan commented May 8, 2022

I'm slightly uneasy about simplifications like these. Are you absolutely sure that this can't break user programs? I wonder if turning an unset to null can matter, for example.

@scop
Copy link
Contributor Author

scop commented May 16, 2022

I cannot think of a case that this could break.

There certainly is a difference between unset and null valued variables (for example [[ -v varname ]] in bash), but I can't see how this simplification could be used to turn a null to an unset or the other way around -- as said, the expanded value (which is what this simplification is about, it's not changing the variable's value or definedness) is null either way for both unset and null valued variable expansions.

@mvdan
Copy link
Owner

mvdan commented May 23, 2022

Alright, SGTM. We can always revert if our thinking here is wrong.

@mvdan mvdan merged commit e8ee47f into mvdan:master May 23, 2022
@scop scop deleted the unset-or-null-null-default branch May 23, 2022 19:42
tomwassenberg added a commit to tomwassenberg/certbot-ocsp-fetcher that referenced this pull request Dec 12, 2022
ericswanson-dfinity added a commit to dfinity/sdk that referenced this pull request Dec 14, 2022
Example failed workflow: https://github.com/dfinity/sdk/actions/runs/3697173489/jobs/6262516972

This is the change to shfmt 3.6.0: mvdan/sh#849
  - Simplify ${name:-} to the equivalent ${name-} - #849
mergify bot pushed a commit to dfinity/sdk that referenced this pull request Dec 15, 2022
Example failed workflow: https://github.com/dfinity/sdk/actions/runs/3697173489/jobs/6262516972

This is the change introduced in shfmt 3.6.0: mvdan/sh#849
  - `Simplify ${name:-} to the equivalent ${name-}` - [#849](mvdan/sh#849)
Abirdcfly added a commit to Abirdcfly/arbiter that referenced this pull request Dec 22, 2022
AdamVig added a commit to AdamVig/dotfiles that referenced this pull request Jan 27, 2023
nkwangleiGIT pushed a commit to nkwangleiGIT/arbiter that referenced this pull request Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants