Skip to content

Conversation

@kini
Copy link

@kini kini commented Oct 15, 2021

The behavior of interpreting "\n" inside quoted strings in env files
was neither documented nor tested, so this commit modifies the unit
tests to pin down that behavior.

I also added a test for the example I'm proposing to add to the docs
in PR microsoft/vscode-docs#4888.

I also extended the test for escaping "$" to show that it works even
when the "$" isn't part of something that matches
SUBST_REGEX (i.e. isn't the beginning of something looking like
"${...}").

The behavior of interpreting "\n" inside quoted strings in env files
was neither documented nor tested, so this commit modifies the unit
tests to pin down that behavior.

I also added a test for the example I'm proposing to add to the docs
in PR microsoft/vscode-docs#4888.

I also extended the test for escaping "$" to show that it works even
when the "$" isn't part of something that matches
SUBST_REGEX (i.e. isn't the beginning of something looking like
"${...}").
@kini kini force-pushed the test-newline-subst branch from 14b76e2 to a4232cd Compare October 15, 2021 07:19
@karrtikr karrtikr closed this Jan 11, 2022
@karrtikr karrtikr reopened this Jan 11, 2022
@karrtikr karrtikr self-requested a review January 11, 2022 19:49
Copy link

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

@kini Apologies for the late response and thanks for submitting this PR! I'll just make sure tests are passing before merging it.

@kini
Copy link
Author

kini commented Jan 11, 2022

Thanks. Keep in mind that this PR adds a test that claims it is testing an example from the docs, but the docs won't actually contain that example until microsoft/vscode-docs#4888 is merged. But that PR is closed because I wasn't sure if what I wrote in it was accurate - I'd appreciate it if you could take a look over there too.

@karrtikr karrtikr self-requested a review January 11, 2022 20:01
@karrtikr karrtikr added the investigating We are looking into the cause of the issue label Jan 12, 2022
Copy link

@karrtikr karrtikr left a comment

Choose a reason for hiding this comment

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

@kini Thanks, you were spot on with your comments in microsoft/vscode-docs#4888 (comment), recursive substitution only works with ${...} whereas ${env:...} is only used for process env variables, so it seems documentation is inaccurate at the moment. Feel free to fix it and add the example, meanwhile I'll create an issue clarifying the usage and link it here soon, We should probably only follow the standard python-dotenv syntax on this: https://pypi.org/project/python-dotenv/

@karrtikr karrtikr merged commit 2c22cbf into microsoft:main Jan 18, 2022
@karrtikr karrtikr removed the investigating We are looking into the cause of the issue label Jan 18, 2022
@karrtikr
Copy link

FYI I have created #18307 where I have clarified how things currently are.

wesm pushed a commit to posit-dev/positron that referenced this pull request Mar 28, 2024
…vscode-python#17747)

The behavior of interpreting "\n" inside quoted strings in env files
was neither documented nor tested, so this commit modifies the unit
tests to pin down that behavior.

I also added a test for the example I'm proposing to add to the docs
in PR microsoft/vscode-docs#4888.

I also extended the test for escaping "$" to show that it works even
when the "$" isn't part of something that matches
SUBST_REGEX (i.e. isn't the beginning of something looking like
"${...}").
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 this pull request may close these issues.

2 participants