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

feat: Add toYamlWithIndent template function #3875

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

twpayne
Copy link
Owner

@twpayne twpayne commented Jul 21, 2024

Fixes #3873.

@vbrand1984 is this sufficient to solve your problem?

@twpayne twpayne requested a review from halostatue July 21, 2024 19:36
@twpayne twpayne marked this pull request as draft July 21, 2024 20:34
@twpayne twpayne requested a review from bradenhilton July 21, 2024 20:34
@twpayne
Copy link
Owner Author

twpayne commented Jul 21, 2024

Converting to draft and adding @bradenhilton as a reviewer re #3873 (reply in thread). Excellent comment.

@bradenhilton
Copy link
Collaborator

I haven't thought it through thoroughly, but could it be better implemented as a template directive, along with JSON and TOML, etc.?

@halostatue
Copy link
Collaborator

I haven't thought it through thoroughly, but could it be better implemented as a template directive, along with JSON and TOML, etc.?

Hmm. TOML officially has no indenting (whitespace before key names is ignored). Sprig provides two functions toJson and toJsonPretty (or whatever it's called), because JSON also has no indenting. YAML does have syntactically significant indenting, and for a given indentation level, all keys must be at the same level.

I’m rambling here, because as nice as this would be, I don't think that either a directive or a new function really does the right thing — and the new function doesn't fix a problem with trying to render YAML inside a nested key (it wasn't requested, but is a bigger problem than the default indent level).

To solve the original problem, I wonder if this might not just be better done as an option? yamlIndent = 2 changes all toYaml outputs to two space indent the same way as this or a directive would.

With respect to "rendering in a nested key", I don't have any immediate examples as I’ve been trying to simplify my templates some, and there are issues with using toYaml for any level other than top-level, at which point you would be better off doing key: {{ .value | toJson }} (since YAML is a JSON superset).

$ chezmoi execute-template 'key:
  value: {{ dict "a" "1" "b" "2" | toYaml}}'
key:
  value: a: "1"
b: "2"

Compared to:

$ chezmoi execute-template 'key:
  value: {{ dict "a" "1" "b" "2" | toJson }}'
key:
  value: {"a":"1","b":"2"}

Using indent doesn't really help unless you do something like:

$ chezmoi execute-template 'key:
  value:
{{ dict "a" "1" "b" "2" | toYaml | indent 4}}'
key:
  value:
    a: "1"
    b: "2"

TL;DR: toYaml should only be used at the top level of a file or where you know the fragment can properly be indented using indent X. Anywhere else, it is exceedingly unsafe. With respect to indentation, I think that a global setting would be better than either a directive or a new template function, but would support a directive over a new template function as well. As much as the new template function makes certain things easier, it does not make toYaml safer to use in any way.

@bradenhilton
Copy link
Collaborator

@halostatue I disagree with it being an option (at least instead of a directive). An option would be nice to provide a default for all files, though.

Most of the time the formatting isn't important (with YAML being an exception to this, as you pointed out), which makes the files where it is important outliers, so to me it makes more sense to have it be a directive which is file-secific.

Just to be clear, I wasn't suggesting adding a directive for each language, rather a more general

chezmoi:template:indent=" "   # Maybe chezmoi:template:format:indent?
chezmoi:template:indent=2
chezmoi:template:indent="\t\t"

or similar, though I am aware that some languages don't support tab indentation, which would possibly require a directive for tab sizes that could be used to convert them to spaces.

@halostatue
Copy link
Collaborator

@halostatue I disagree with it being an option (at least instead of a directive). An option would be nice to provide a default for all files, though.

Most of the time the formatting isn't important (with YAML being an exception to this, as you pointed out), which makes the files where it is important outliers, so to me it makes more sense to have it be a directive which is file-secific.

Just to be clear, I wasn't suggesting adding a directive for each language, rather a more general

chezmoi:template:indent=" "   # Maybe chezmoi:template:format:indent?
chezmoi:template:indent=2
chezmoi:template:indent="\t\t"

I can buy that, especially with chezmoi:template:format:indent or chezmoi:template:format-indent as it would suggest that it is for use with formatters (e.g., toYaml, toToml, toJson, toPrettyJson).

@twpayne twpayne force-pushed the master branch 2 times, most recently from c8ae27a to 7a68dfc Compare August 5, 2024 23:55
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.

3 participants