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

Implement shell output format #1645

Merged
merged 6 commits into from May 4, 2023
Merged

Conversation

giorgiga
Copy link
Contributor

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 print value=somevalue.

I've not updated the documentation yet.

@mikefarah
Copy link
Owner

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

@giorgiga
Copy link
Contributor Author

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.

@giorgiga
Copy link
Contributor Author

Forgot to mention: the makefile doesn't work for me...

I get (for example):

$  make vendor
/usr/bin/podman run --rm -e LDFLAGS="-X main.GitCommit=6017174 -X main.GitDescribe=v4.33.3-6-g6017174 -w -s" -e GITHUB_TOKEN="" -v /home/giorgio/projects/yq/vendor:/go/src -v /home/giorgio/projects/yq:/yq/src/github.com/mikefarah/yq -w /yq/src/github.com/mikefarah/yq yq_dev go mod vendor
✔ docker.io/library/yq_dev:latest
Trying to pull docker.io/library/yq_dev:latest...
Error: initializing source docker://yq_dev:l

I run the scripts/ directly without a container so I guess it doesn't matter for this PR, but I thought you might have wanted to know (as for what the problem is and if it's the makefile or my machine, I have no idea)

@mikefarah
Copy link
Owner

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.

return err
}

func (pe *shellVariablesEncoder) doEncode(w *io.Writer, node *yaml.Node, path string, _ *yaml.Node) error {
Copy link
Owner

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?

Copy link
Contributor Author

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!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 8902f98

}

func isAlphaOrUnderscore(r rune) bool {
return ('a' <= r && r <= 'z') || ('A' <= r && r <= 'Z')
Copy link
Owner

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 == '_' ?

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 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 _.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 8902f98

@mikefarah mikefarah merged commit 80b42b8 into mikefarah:master May 4, 2023
3 checks passed
@giorgiga giorgiga deleted the issue-1632 branch May 4, 2023 10:17
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.

None yet

2 participants