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
os: Expand eats $s in the middle of the string (but not at the end) #24345
Comments
Hello This seems an side effect os this https://golang.org/pkg/os/#ExpandEnv
a variable is defined as so https://golang.org/src/os/env.go#L26 will always remove variables even if it is not a valid variable ( like remove in fact the documentation omit few things:
It seems pretty KISS: if you try to expand and a single $ is removed with no warning this code can review part of the inner logic: IMHO we may describe this behavior a little bit better on the documentation. To fix your issue: maybe you can use two templates:
this give you the ability to validate or escape the expanded string if needed |
To be consistent, I think the trailing $ needs to be removed too. The first $ is correctly gobbled because | is neither a shell special variable nor an alphanumeric character. The expected output, IMO, should be Moreover It's a relatively small change. /cc @ianlancetaylor for decision. |
Correct me if I'm wrong, but with your proposed change nobody would be able to use a literal
|
I believe that the correct way to fix it is to leave
|
My proposal does not change that behavior. It is already the existing behavior. My proposal was to make it consistent. Theoretically, I agree with your proposal, but I am not sure how much of a breaking change is this considered to be. I will leave it to others to make the call. |
Thats sounds incorrect. The bug is that The documentation says that os.Expand replaces the |
@as |
@opennota I don't expect this behavior either. I expect all tokens denoting a potential variable name to be excised after one pass of Besides, your desired behavior is trivial to implement: just make your own |
For the reference, in both Python 2 and 3 os.path.expandvars doesn't even touch variables with non-alphanumeric names:
That's the most sane behaviour, IMO. |
This issue seems to be about two different things. The first is that The second problem in this issue is that |
How about allow escape $ as literal $ ? |
I try to wrote \$ sorry |
@peczenyj First let me repeat that I think there there are two separate problems here. Which one are you trying to address? That said, I don't agree with adding backslash quoting to either case. It's not necessary, because we can use |
@ianlancetaylor |
The cgo pointer passing rules were different, as use of cgo is not covered by the Go 1 compatibility guarantee. The current |
Why not? It was never officially supported ( Alternatively, allow only a limited subset of characters to be treated as a variable, otherwise leave |
Let's not change existing behavior unless we are solving a clear problem. We already check for a list of special characters; is |
We should be consistent. It seems that keeping the $ in both cases is ideal rather than eliminating it only at the end. |
@spf13 - So just to be clear, you want $ to remain in the output in case the following character is neither a shell special variable nor alphanumeric . So, |
Yes. |
Cool. Will send a CL. |
Change https://golang.org/cl/103055 mentions this issue: |
What version of Go are you using (
go version
)?go version go1.10 linux/amd64
Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?What did you do?
https://play.golang.org/p/ulxC4AgmVNF
What did you expect to see?
The program outputs
abc$|def$
What did you see instead?
The program outputs
abc|def$
This is unfortunate, because
go generate
usesos.Expand
to expand environment variables in the command line and if one needs to pass a regexp to the executed command, the regexp gets garbled.The text was updated successfully, but these errors were encountered: