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

support MODE env to have spaces #22

Open
marc-portier opened this issue Jun 10, 2024 · 0 comments · May be fixed by #29
Open

support MODE env to have spaces #22

marc-portier opened this issue Jun 10, 2024 · 0 comments · May be fixed by #29

Comments

@marc-portier
Copy link

This fix --> bb0afbf

introduced this line

if [ ${MODE:-""} == "development" ]; then

most likely to prevent the warning

unset MODE
/start.sh
./start-bad.sh: line 4: [: =: unary operator expected

because when MODE is unset the old approach lead the sh parser to trip over the test-syntax becoming [ = "development"] rather then [ "" = "development" ]

The more natural / elegant / simple / common strategy to this problem however is to simply use:

if [ "${MODE}" == "development" ]; then 

This has the added benefit of even supporting MODE values that have spaces in them. Cause in the current setup one would still have the sh parser trip over a use case like

export MODE="my mode"
/start.sh
./start-rpio.sh: line 4: [: too many arguments

simply because the test-syntax ends up as [ my mode = "development" ] rather then the hoped for [ "my mode" = "development" ]

I understand having MODE values with spaces is pushing beyond normal expectations, but as said, the suggested fix is also the more readable and common way of doing things.

nvdk added a commit to nvdk/mu-python-template that referenced this issue Nov 18, 2024
fixes mu-semtech#22 

As mentioned there  in the current setup one would still have the sh parser trip over a use case like
```
export MODE="my mode"
./start.sh: line 4: [: too many arguments
```
@nvdk nvdk linked a pull request Nov 18, 2024 that will close this issue
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 a pull request may close this issue.

1 participant