Skip to content
This repository has been archived by the owner on Jun 16, 2021. It is now read-only.

Support a-z and 0-9 in variable names #70

Merged
merged 1 commit into from Oct 7, 2015

Conversation

ibuildthecloud
Copy link
Contributor

Signed-off-by: Darren Shepherd darren@rancher.com

@ibuildthecloud
Copy link
Contributor Author

@dnephin Can you confirm that the list of valid character is correct? It is now a-z, A-Z, _ and 0-9.

@dnephin
Copy link
Contributor

dnephin commented Oct 7, 2015

r'[_a-z][_a-z0-9]*' is the pattern (used with _re.IGNORECASE), so I believe that is correct

testInterpolatedLine(t, "WORKED", "$lower", variables)
testInterpolatedLine(t, "WORKED", "${MiXeD}", variables)
testInterpolatedLine(t, "WORKED", "${split_VaLue}", variables)
testInterpolatedLine(t, "WORKED", "$9aNumber", variables)
Copy link
Contributor

Choose a reason for hiding this comment

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

In the python implementation this wouldn't work. I think the reason is that it's using the same logic as bash variables, which can't start with a number

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I can make thing starting with a number not work, but an embedded number would be fine?

Signed-off-by: Darren Shepherd <darren@rancher.com>
@ibuildthecloud
Copy link
Contributor Author

@dnephin Updated to not allow starting with a number

@thaJeztah
Copy link
Member

Is this for env-vars? You may be interested in moby/moby#16608 (and the linked issue)

@dnephin
Copy link
Contributor

dnephin commented Oct 7, 2015

@thaJeztah nope, not for env-vars, this is for variables in the compose file (it just happens we use very similar syntax).

@@ -8,6 +8,17 @@ import (
"github.com/Sirupsen/logrus"
)

func isNum(c uint8) bool {
return c >= '0' && c <= '9'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It still kind of surprises me that there is no golang builtin for super common operations like this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The joys of golang.

@dnephin
Copy link
Contributor

dnephin commented Oct 7, 2015

LGTM

ibuildthecloud added a commit that referenced this pull request Oct 7, 2015
Support a-z and 0-9 in variable names
@ibuildthecloud ibuildthecloud merged commit 1df7ca4 into docker:master Oct 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants