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

Adding configuration variable replacement in c_cpp_properties.json. #1529

Merged
merged 3 commits into from
Feb 7, 2018

Conversation

thejcannon
Copy link
Contributor

As requested in issue #314 in addition to "env.ENV_VAR", "env:ENV_VAR" and just "ENV_VAR", adding support for "config:config_variable" and "config.config_variable".

@msftclas
Copy link

msftclas commented Feb 7, 2018

CLA assistant check
All CLA requirements met.

let ret: string = input.replace(regexp, (match: string, ignored1: string, varType: string, ignored2: string, name: string) => {
// Historically, if the variable didn't have anything before the "." or ":"
// it was assumed to be an environment variable
if (varType === undefined) { varType = "env"; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

varType = "env"; } [](start = 37, length = 18)

I believe our linting rules require the body of the if to be on a new line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran TSLint and got "vscode-tslint: Status is OK" (It did find some other lint issues though).
Happy to move it to new line, though!

Copy link
Member

@bobbrow bobbrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@bobbrow
Copy link
Member

bobbrow commented Feb 7, 2018

@thejcannon, I'm ready to accept this pull request. You just need to sign the CLA if you haven't already then I can click the button.

Thanks for the help!

@thejcannon
Copy link
Contributor Author

@bobbrow Just waiting on approval from my employer.

@thejcannon
Copy link
Contributor Author

@bobbrow done!

@bobbrow bobbrow merged commit d5af9a1 into microsoft:master Feb 7, 2018
@bobbrow
Copy link
Member

bobbrow commented Feb 7, 2018

Awesome. Thanks again!

@thejcannon thejcannon deleted the config_replacement branch February 7, 2018 18:50
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants