-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fixes for environment variables sent to components #74
Conversation
- add comment for get_sharedenv function - get-launch-components now gets the environment variables - start-component trims values of environment variables. Signed-off-by: Pablo Hernán Carle <[email protected]>
Signed-off-by: Pablo Hernán Carle <[email protected]>
Signed-off-by: Pablo Hernán Carle <[email protected]>
Signed-off-by: Pablo Hernán Carle <[email protected]>
Signed-off-by: Pablo Hernán Carle <[email protected]>
strcat(output, " "); | ||
} else { | ||
strcat(output, "=\""); | ||
strcat(output, envValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quetos are not escaped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean here, this code is from the previous version, just moved it up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON content is solved by jsonToString
. The potential issue could come from shared_uss_env[idx++] = thisEnv;
.
The proper solution there should probably be like this:
if (*thisEnv != '\"') {
newstr = malloc(?);
newstr[0] = 0;
strcat(newstr, key);
strcat(newstr, "=");
strcat(newstr, escapeString(thisEnv + length);
length = newstr;
}
...anyway, I guess it is a bug, that could be solved in an issue. The workaround could be to use a different form in STC. And from another hand, it is pretty rear to set especially in this way string value with a quote.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opened an issue based on testing done with these scenarios #76
Signed-off-by: Pablo Hernán Carle <[email protected]>
Thank you, this is looking good to me as well so I'm going to merge it due to @pj892031 's approval. |
_CEE_ENVFILE*
are not sent downwards to the components to avoid conflicts.zwe internal get-launch-components
Resolve #75