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

Leading and trailing whitespace for values in starter.ini can break the system #174

Closed
apolopena opened this issue Jan 21, 2022 · 1 comment
Assignees
Labels
bug Something isn't working passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main

Comments

@apolopena
Copy link
Owner

apolopena commented Jan 21, 2022

Describe the bug

Putting leading or trailing whitespace in a value in starter.ini can cause hard to track bugs
Ideally all leading or trailing whitespace should be removed from values when they are parsed but realistically for interim the leading and trailing whitespace values may just need to be trimmed after they are parsed.

Screenshots

Steps to reproduce

in starter.ini add some trailing whitespace to version=7.4 in the [PHP] section
The php version will now be invalid and will fall back to 'latest'

Expected behavior

Leading or trailing whitespace should not break the value

Additional information

This problem could happen in alot of places so it should probably be fixed in the parsing utility rather than everywhere a value is parsed. Although doing it this way would make the parsing function less 'multipurpose'.
Perhaps a flag can be passed to the function as a third argument that would preserve all whitespace.

See the parse_ini_value function in https://github.com/apolopena/gitpod-laravel-starter/blob/main/.gp/bash/utils.sh for the parsing utility function.

@apolopena apolopena added bug Something isn't working refactor labels Jan 21, 2022
@apolopena apolopena self-assigned this Jan 21, 2022
@apolopena
Copy link
Owner Author

Currently I dont have enough time to properly test this fix if I make the change in parse_ini_value.

So I am going to add a trim_external function to utils.sh and then just use it trim values after they are parsed.

Later as an optimization and when I have the time to test it. I will add a flag as a third argument to the parse_ini_value in utils.sh that will preserve all trailing and leading whitepspace.

Note:
Testing stuff like this is really time consuming because of how gitpod builds its docker images and the fact that parsing ini values is pretty much the core function of gitpod-laravel-starter.

@apolopena apolopena changed the title Leading and trailing whitespace should be stripped from all values in starter.ini Leading and trailing whitespace for values in starter.ini can break the system Jan 21, 2022
apolopena added a commit that referenced this issue Jan 21, 2022
@apolopena apolopena added this to the v1.4 release 🚀 milestone Jan 21, 2022
@apolopena apolopena added passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main and removed in-dev-qa labels Jan 21, 2022
apolopena added a commit that referenced this issue Jan 22, 2022
@apolopena apolopena added in-dev-qa passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main and removed passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main in-dev-qa labels Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working passed-dev-qa Optional state. Use this when QAing other peoples fixes in another branch.ready to be merged to main
Projects
None yet
Development

No branches or pull requests

1 participant