-
-
Notifications
You must be signed in to change notification settings - Fork 616
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
Implement shell output format #1645
Conversation
Looks good! For testing (and documentation) - can you follow the pattern in TestPropertyScenarios (albeit you'll only have encode tests). That will also generate documentation |
Thanks! I added the tests but couldn't figure out how to generate the actual html documentation (the markdown seems ok however). Please review and see if the way I named things makes sense and is in harmony with the rest of the project. Also, please let me know if you want me to squash the commits in this PR (and how) or if you are going to squash when merging. |
Forgot to mention: the makefile doesn't work for me... I get (for example):
I run the |
Hmm - I run everything locally (make local build) rather than docker/podman. Markdown is all that's needed :) The service I use for documentation does the conversion to HTML itself. |
pkg/yqlib/encoder_shellvariables.go
Outdated
return err | ||
} | ||
|
||
func (pe *shellVariablesEncoder) doEncode(w *io.Writer, node *yaml.Node, path string, _ *yaml.Node) error { |
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.
Not sure why you have the last argument here if it's just ignored?
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.
It's vestigial remains from the properties encoder that I initially cop-ypasted - good catch!
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.
Fixed with 8902f98
pkg/yqlib/encoder_shellvariables.go
Outdated
} | ||
|
||
func isAlphaOrUnderscore(r rune) bool { | ||
return ('a' <= r && r <= 'z') || ('A' <= r && r <= 'Z') |
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.
doesn't this need || r == '_'
?
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 function was initially just isAlpha
... I must have thought renaming was all it needed to change the implementation!
I'm adding a test case case for this... well, for when the root key starts with an underscore, which is why this function should allow _
.
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.
Fixed with 8902f98
This implements the idea at #1632 to export in a format that can be sourced into bash/sh.
Since both yaml keys and shell variables are case-sensitive, it seems converting to uppercase as I imagined while writing #1632 is not the best of ideas after all 😅.
If case is preserved, there are instances where the yaml keys will not correspond to the exported variable (because variable names are quite restrictive compared to yaml). see the comments in
encoder_shellvariables.go
for details.Eg.
echo -e '1:\n "a a": somevalue' | go run yq.go -o shell
will output_1_a_a=somevalue
(note the initial underscore and the underscore that replaces the space between the to 'a').Documents consisting of a single scalar value are converted as if a key named "value" was present. This is because it does not make sense to have variables with empty names (and
_
is a special shell variable that can't be assigned to).Eg.
echo somevalue | go run yq.go -o shell
will printvalue=somevalue
.I've not updated the documentation yet.