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

Yaml multiline string pretty-printing in manifestification #144

Closed
psalaberria002 opened this issue Jan 11, 2024 · 6 comments
Closed

Yaml multiline string pretty-printing in manifestification #144

psalaberria002 opened this issue Jan 11, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@psalaberria002
Copy link

Since 0.9 the string formatting defaults to pretty formatting. See dtolnay/serde-yaml#226.

It would be great to get that updated.

@CertainLach
Copy link
Owner

serde-yaml here is only used for parsing, manifestification is implemented manually, the same way as it implemented in official jsonnet: https://github.com/CertainLach/jrsonnet/blob/master/crates/jrsonnet-stdlib/src/manifest/yaml.rs#L110

It should be possible to implement different string manifestification strategy, but this should be also proposed/implemented on https://github.com/google/jsonnet side.

@CertainLach CertainLach changed the title Update serde_yaml to >=0.9 Yaml multiline string pretty-printing in manifestification Jan 11, 2024
@CertainLach CertainLach added the enhancement New feature or request label Jan 11, 2024
@rbtcollins
Copy link

We get a much better manifestation from kubecfg, which is a go program sitting on top of the go jsonnet and doing https://github.com/kubecfg/kubecfg/blob/main/pkg/kubecfg/show.go#L159-L171 - I think we'd be happy to do a similar shim on top of jrsonnet if we need to, but since producing config files at scale is a key use case, and since readability when doing code review matters - on the output - it would be great if you can suggest some way forward that involves somewhat less redundancy :)

@CertainLach
Copy link
Owner

I'll think about this, I might add this feature under the feature-flag, because I agree such multiline string output is much better.

I want jrsonnet to be mostly output-compatible with go-jsonnet. I won't make it worse in the sake of compatibility (Such as in #108), but I don't want defaults to be different.

@CertainLach
Copy link
Owner

It might be a good idea to leave std.manifestYamlEx as-is, but then add new output format --format=pretty-yaml.
There already exist --format=json (Which is default), --format=yaml (Which is default when using -y flag), --format=string (This is an alternative syntax for -S) and --format=toml

@rbtcollins
Copy link

rbtcollins commented Jan 11, 2024

I think we've found a workaround for now - we just do

   thing: std.manifestYamlDoc(obj_to_stringify) + "\n"

which triggers your codepath here:

} else if let Some(s) = s.strip_suffix('\n') {

@CertainLach
Copy link
Owner

Oh, it already exists in jsonnet:
https://github.com/google/jsonnet/blob/master/stdlib/std.jsonnet#L1219-L1223

        else if v[len - 1] == '\n' then
          local split = std.split(v, '\n');
          std.join('\n' + cindent + '  ', ['|'] + split[0:std.length(split) - 1])
        else
          std.escapeStringJson(v)

Even better, then it can be implemented by default for jrsonnet, and then it is possible to update original jsonnet to include this shorthand as well:

        else if v[len - 1] == '\n' then
          local split = std.split(v, '\n');
          std.join('\n' + cindent + '  ', ['|'] + split[0:std.length(split) - 1])
        else if std.member(v, '\n') then
          local split = std.split(v, '\n');
          std.join('\n' + cindent + '  ', ['|-'] + split[0:std.length(split) - 1])
        else
          std.escapeStringJson(v)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants