fix: correct handling of ASDF_FORCE_PREPEND environment variable
#2011
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The error occurs if I set "yes" to `FORCE_PREPEND` env as described in the documentation. Error message: ``` strconv.ParseBool: parsing \"yes\": invalid syntax ``` Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend
machupicchubeta
force-pushed
the
force_prepend
branch
from
654ba96 to
82c9dc0
Compare
…ironment variable Those test cases aims to check whether `ForcePrepend` was actually set to the correct/expected value. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend
machupicchubeta
force-pushed
the
force_prepend
branch
3 times, most recently
from
559004e to
b017252
Compare
If the `ASDF_FORCE_PREPEND` environment variable is not set on macOS, the behavior is the same as when "yes" is set. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend
`ASDF_FORCE_PREPEND` is a specification that changes behavior depending on whether the string is "yes" or not. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/7352bf4890143184ba419392ca5e6204167c8306/docs/manage/configuration.md#asdf_force_prepend
machupicchubeta
force-pushed
the
force_prepend
branch
from
b017252 to
934f15d
Compare
Stratus3D
reviewed
Mar 14, 2025
| @@ -59,8 +61,13 @@ type Settings struct { | |||
| } | |||
|
|
|||
| func defaultConfig(dataDir, configFile string) *Config { | |||
| forcePrepend := forcePrependDefault | |||
| forcePrependEnv := os.Getenv("ASDF_FORCE_PREPEND") | |||
There was a problem hiding this comment.
We might want to be able to handle inconsistent case here. Some folks might set this to YES by mistake and I think that should be handled.
There was a problem hiding this comment.
I think it means case-insensitive. Sounds good to me.
I'll try to write a commit.
Stratus3D
approved these changes
Mar 14, 2025
There was a problem hiding this comment.
Thanks for the PR @machupicchubeta ! I really appreciate the added tests!
Stratus3D
changed the title
ASDF_FORCE_PREPEND environment variable
Merged
|
@andrecloutier @Stratus3D |
machupicchubeta
changed the title
ASDF_FORCE_PREPEND environment variableASDF_FORCE_PREPEND environment variable
machupicchubeta
changed the title
ASDF_FORCE_PREPEND environment variableASDF_FORCE_PREPEND environment variable
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 19, 2025
I agree that it would be valid even if it is set in uppercase as well as lowercase. @Stratus3D commented: - asdf-vm#2011 (comment) > We might want to be able to handle inconsistent case here. Some folks might set this to YES by mistake and I think that should be handled.
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 19, 2025
I agree that it would be valid even if it is set in uppercase as well as lowercase. @Stratus3D commented: - asdf-vm#2011 (comment) > We might want to be able to handle inconsistent case here. Some folks might set this to `YES` by mistake and I think that should be handled.
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 20, 2025
Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 20, 2025
Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 20, 2025
Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 20, 2025
Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 20, 2025
Previously, the value of `ForcePrepend` was not used anywhere and was not behaving as specified. With this commit, the behavior will be as specified. BREAKING CHANGE: So far, `ASDF_FORCE_PREPEND` environment variable setting has been ignored, but this commit may change the behavior depending on the setting. If it is not set, macOS behavior will not change, but Linux behavior will change. If you want to get the same behavior as before, it is recommended to set the relevant environment variable. Referenced Documents: - https://github.com/asdf-vm/asdf/blob/932ac468b7c24c2adef90a293a1f7280a0074cc4/docs/manage/configuration.md#asdf_force_prepend Background: @andrecloutier told about this issue me in a comment on another PullRequest. Thank you! - asdf-vm#2011 (comment)
machupicchubeta
added a commit
to machupicchubeta/asdf
that referenced
this pull request
Mar 20, 2025
I agree that it would be valid even if it is set in uppercase as well as lowercase. @Stratus3D commented: - asdf-vm#2011 (comment) > We might want to be able to handle inconsistent case here. Some folks might set this to `YES` by mistake and I think that should be handled.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The error occurs if I set "yes" to
FORCE_PREPENDenv as described in the documentation.Error message
Other Information
Referenced Documents