[lang] Support for standard YAML comments

dpb587-pivotal commented 5 years ago

Is there a reason standard YAML comments are not supported? Since ytt seems to try and sort of follow YAML syntax, I expected it to be ignored by ytt.

$ cat > example.yml <<EOF
earlier: former
# later: latter # TODO someday
EOF

$ ytt template -f example.yml
Error: Unknown comment syntax at line example.yml:2: ' later: latter # TODO someday': Unknown metadata format (use '#@' or '#!')

I could imagine #something without a following space to be something ytt doesn't support due to theoretical ambiguity, but #spacesomething seems reasonable for ytt to ignore as standard YAML convention and non-ytt directive-style.

I understand #! is an alternative, but that's just more of a migration/difference and don't think I understand why it should be needed.

(meant to submit as @dpb587 from personal side-project learnings)

cppforlife commented 5 years ago

our current thinking was to be more cautious and disallow plain comments (# blah) since users may forget to add @ after # (#@ blah) and their code would not run. i've definitely been a bit annoyed by it as well. i've added file-mark to be able to specify that particular file is plain (non-templated yaml)

ytt t -R -f . --file-mark file1.yml:type=yaml-plain

we may at some point relax this restriction, and just ignore #. @nimakaviani wdyt about that?

nimakaviani commented 5 years ago

yeah even though it is annoying to have the interpreter yell at you because of using standard yaml comments, that appeared to be the safer choice compared to rendering some of the templating code useless.

As for the file-mark flag, i like the idea however not totally fond of the syntax. A less verbose option with dedicated flags may be slightly easier from a ui perspective.

cppforlife commented 5 years ago

@dpb587-pivotal wdyt?

dpb587 commented 5 years ago

Thanks, I understand the concerns. From my perspective, ytt seems additive to YAML so I'd hoped standard YAML would not need additional work. In terms of users forgetting, I'd expect them to quickly notice if their ytt operations are not taking effect as they're updating and testing, so with ytt requiring #!/#@-type directives, I'd assume it to be fairly discoverable. Alternatively a less verbose form of --debug could emit warnings for syntax hints if ytt were to accept standard comments.

Regarding file-mark, I hadn't realized its use yet, so it's good to know. Although I, too, think it's a little difficult to realize or confusing.

Feel free to close if you don't see any behaviors needing to change.

cppforlife commented 5 years ago

ill leave the issue open for now for more thinking to be done.

mildred commented 5 years ago

I'm hitting this as well, and not being able to use plain YAML comments seems counter intuitive.

Alternative to--file-mark would be to annotate at the beginning of the file something like:

#@ytt/file-mark type:yaml-plain

Because it belongs to the file, not the caller.

cppforlife commented 5 years ago

for now added --ignore-unknown-comments=bool in https://github.com/k14s/ytt/releases/tag/v0.12.0

cppforlife commented 5 years ago

@mildred i think its a good idea to have something like that; however, it wouldnt help in cases when you are trying to avoid modifying files (they come from upstream)

patricknelson commented 4 years ago

It's not always essential for the resulting YAML to have comments for me, but it certainly would be very helpful!

What if we were to use a double # to work as an escape method when at the beginning of a line, so that the resulting YAML will contain a standard comment? For example, taken from the main demo at https://get-ytt.io:

#@ def labels():
#! This is a ytt comment
## This is a yaml comment
app: echo
org: test
#@ end

kind: Pod
apiVersion: v1
metadata:
  name: test-comments
  labels: #@ labels()

With the output resulting in:

kind: Pod
apiVersion: v1
metadata:
  name: echo-app
  labels:
    # This is a yaml comment
    app: echo
    org: test

I'm brand new to ytt (just doing research right now), so I'm asking just in case this isn't already available with first class syntax support without any special flags.

cppforlife commented 4 years ago

@patricknelson moved your use case into a dedicated issue #63.

aaronshurley commented 4 years ago

[ ] Document intent behind comment syntax
[ ] Consider using a hint for when a single # is detected. The hint could include the intent.

jessehu commented 4 years ago

Can we make --ignore-unknown-comments=true as the default? BTW the term unknown comments seems a little confusing, as it's a known comment (starting with '#' ) which follows yaml spec.

cppforlife commented 4 years ago

Can we make --ignore-unknown-comments=true as the default?

see our reasoning behind this: https://github.com/k14s/ytt/issues/14#issuecomment-485489527. still evaluating, so not rushing on a decision to relax constraint.

BTW the term unknown comments seems a little confusing, as it's a known comment (starting with '#' ) which follows yaml spec.

may be "non-qualified comment" is a better term?

jorgemoralespou commented 4 years ago

What's wrong with regular "comment"?

If you think on adding a qualifier to the route of comment, does it mean that there will be flags to ignore the other "types" of comments?

Did it even make sense to add ytt (#!) or ytt-yaml (##) comments if they are added explicitly by the user/template author?

By the way, I also would love to have --ignore-comments=true being the default. This annoys new users, while experience ones would know how to provide a flag to ignore-comments=false. Making simplicity of use the default for non power users would improve adoption.

jessehu commented 4 years ago

I think this term is also conflict with the ytt principle that all ytt template yaml file should be a valid yaml file: may be "non-qualified comment" is a better term. Maybe '--ignore-yaml-comments' and default to true ?

jessehu commented 4 years ago

IMHO ytt as a competitor to helm and helm chart templates, should have less incompatibility or concept change to a user who want to switch from helm to ytt/kapp.

I vote for Making simplicity of use the default for non power users would improve adoption.

jtigger commented 3 years ago

I have now had many conversations with users who have found many little sharp edges in the UX of ytt like this to be a kind of "death by a 1,000 cuts" experience... to the point where some are reporting that the tool can feel outright hostile.

It's becoming my understanding that the views expressed in this issue are widely held. And that we have crossed a threshold from "safe by default" to "pedantic to a frustration."

I have yet to hear anyone who was grateful for this default.

pivotaljohn commented 3 years ago

Update (from conversations around #226)

TL;DR.

We'll take the message improvements. In a separate feature request, we'll teach ytt to detect which YAML files are templates and which are not. Once that is in place, we can toggle the default for --ignore-unknown-comments. The net experience will be the effective deprecation of this flag without losing the linting of YAML templates.

Details

There are two types of YAML files in ytt:[1]

yaml-plain which are parsed, but not processed for templating.
yaml-template which are parsed and processed for templating.

The sorts of YAML that are part of the primary pain around this feature are "plain YAML" that are being treated like "yaml-template". As @cppforlife pointed out, previously, one can correct this using a file mark. However, that solution can be considered cumbersome.

As a patch over the problem, the --ignore-unknown-comments was introduced to disable detecting when a Configuration Author might have accidentally omitted an annotation (i.e. started a line that's meant to be template with # (note the space) instead of #@). As @cppforlife has explained, the longer flag name is meant to signal to the user that they are possibly doing something dangerous (here, disabling that check).

The need for this flag has flummoxed many a user (if not downright irritated them).

This is where this suggestion comes into play:

introduce smarter detection in ytt for plain vs templated file

if we see one #@ or #! comment then consider file as template, otherwise plain yaml

With this feature in place, 95%+ of the cases are covered:

3rd party YAML (i.e. not ytt templates) will be accurately set as yaml-plain and can contain whatever valid YAML they wish (including comments).
ytt templates continue to enjoy a check where if the author makes an annotating typo, we catch it.
in the odd case where a 3rd party YAML contains a comment that starts with #@, file marks can be used to override this automatic detection.

With YAML file type detection in place, the --ignore-unknown-comments is obviated:

the presence of the flag will result in a deprecation warning.
once the flag has almost completely fallen out of use, we'll remove it.

[1] https://github.com/k14s/ytt/blob/develop/docs/file-marks.md

jessehu commented 3 years ago

@jtigger & @cppforlife Thanks a lot for the detailed analysis 👍

patricknelson commented 3 years ago

# (note the space)

I presume we'd expect a similar failure scenario as in in @davedittrich's use case as well (e.g. #cloud-config), not just hash-spaces, correct?

introduce smarter detection in ytt for plain vs templated file

if we see one #@ or #! comment then consider file as template, otherwise plain yaml

It appears this has morphed from being "Support standard YAML comments" (I suppose treating # Comment as #! Comment without erroring) into how to treat an entire file (e.g. yaml-plain vs yaml-template). For me, my 95% use case will be yaml-template where the full file (or pipe) is originally intended for ytt, so the allowance of # Comment is simply a succinct syntactic sugar that has the side effect of also working normally on plain non-ytt formatted YAML code (be it just a segment of plain YAML or an entire file, a.k.a. yaml-plain).

That's why I like @jessehu's suggestion in https://github.com/k14s/ytt/pull/226#issuecomment-721432076

We can also rename --ignore-unknown-comments to --use-strict-comments with default value false. Similar to "use strict" in Javascript.

That is, being less "safe" by default but still retaining linting capability by adding a new --use-strict-comments flag (and/or similar file marker). Of course, this would be a non-backward compatible change due to the change in default behavior and how it'd affect developer workflows. However, that approach is at least core to the spirit of the original submitter's issue (Support standard YAML comments). I only bring this up because I'm catching up to the thread and I was confused by this approach (POLA may apply here) and also because that suggestion should at least be mentioned here as well. 😄

patricknelson commented 3 years ago

Maybe we could also summarize and discuss the expected behavior (along with explanations for those decisions) with regard to # and ytt, in all of its permutations? That is:

# We consider this ambiguous. Did you mean to comment only visible in ytt, emit it in output YAML or miss an @? To ignore (and treat as a ytt comment, pass --ignore-unknown-comments)
#Same as above. Note the lack of space.
## Explicitly intended to be emitted in resulting YAML output (pending issue #63)
#! Explicitly intended to be a comment only visible in ytt.
#@ Explicitly intended for ytt directives

The above is just an example of the expectations, of course. I know that sort of guidance would help me a lot in understanding what ytt expects of me when passing code to it. Once settled, it'd be great to have that somewhere easily visible in documentation early on somehow so it's easier to understand.

pivotaljohn commented 3 years ago

I presume we'd expect a similar failure scenario as in in @davedittrich's use case as well (e.g. #cloud-config), not just hash-spaces, correct?

Absolutely — in effect, when a YAML file becomes a yaml-template, ytt is taking over the commenting "space" in that document. Any data in that space that isn't recognized by ytt is a potential template coding error.

It appears this has morphed from being "Support standard YAML comments" (I suppose treating # Comment as #! Comment without erroring) into how to treat an entire file (e.g. yaml-plain vs yaml-template).

Yeah, that shift happened. There are a number of use-cases we're solving for simultaneously. In an attempt to keep things as simple as possible, we're chunking down for a more fundamental shift to try to accommodate as many as we can.

For me, my 95% use case will be yaml-template where the full file (or pipe) is originally intended for ytt, so the allowance of # Comment is simply a succinct syntactic sugar that has the side effect of also working normally on plain non-ytt formatted YAML code (be it just a segment of plain YAML or an entire file, a.k.a. yaml-plain).

We hear and appreciate that the original request was asking for allowing YAML comments in ytt templates. We'd love to be able to treat these inputs this way. However, as detailed above, allowing for such comments (unfortunately) also has the side effect of allowing for subtle template code errors (best illustration is where there's a lot of annotations and just one line or another is missing the meta-character: the result is that one line is quietly not executed).

It's hard not to see how when the tool does support plain YAML and that there's a notation for commenting in templates that it's reasonable to require authors to use that notation. It's legit possible we're missing something — if so, please help us understand.

That is, being less "safe" by default but still retaining linting capability by adding a new --use-strict-comments flag (and/or similar file marker).

Being less safe by default is an anti-goal for this project. This recent discussion has been all about how to retain that characteristic while improving the UX around widely held needs (namely, flowing plain YAML through the tool).

Of course, this would be a non-backward compatible change due to the change in default behavior and how it'd affect developer workflows.

It's hard to exaggerate the costs of breaking changes. We'd really want to be making a sweeping improvement that benefits the vast majority of our community to do so.

pivotaljohn commented 3 years ago

Maybe we could also summarize and discuss the expected behavior (along with explanations for those decisions) with regard to # and ytt, in all of its permutations? That is:
# We consider this ambiguous. Did you mean to comment only visible in ytt, emit it in output YAML or miss an @? To ignore (and treat as a ytt comment, pass --ignore-unknown-comments)
#Same as above. Note the lack of space.
## Explicitly intended to be emitted in resulting YAML output (pending issue #63)
#! Explicitly intended to be a comment only visible in ytt.
#@ Explicitly intended for ytt directives
The above is just an example of the expectations, of course. I know that sort of guidance would help me a lot in understanding what ytt expects of me when passing code to it. Once settled, it'd be great to have that somewhere easily visible in documentation early on somehow so it's easier to understand.

Excellent suggestions, @patricknelson; thank you.

Yeah, a straight-forward authoring guide seems in order. A key (and prominent) section would handle exactly this notation.

DanielJonesEB commented 3 years ago

May I politely suggest that upholding the principle of least astonishment is paramount for widespread adoption. I understand the decisions made up until this point, and the proposed remedy sounds good, but "your (perfectly valid) document isn't right" sends a bad message to new adopters, who are probably using ytt in part because they can't change the upstream document.

It'd be great if the politeness of the tool to new users is prioritised in future. It'd be a terrible shame for such a useful and powerful thing suffer similar problems to BOSH. This suggestion is offered in the spirit of ensuring that your hard work results in successful adoption, and comes from a position of admiration and respect.

gcheadle-vmware commented 3 years ago

@DanielJonesEB Thank you for these thoughts, I completely agree that our tools should be predictable and not surprise users. One of our goals for the upcoming work in ytt is to make the tool easier to use for beginners. So, we very much appreciate your thoughts on this topic.

patricknelson commented 3 years ago

However, as detailed above, allowing for such comments (unfortunately) also has the side effect of allowing for subtle template code errors (best illustration is where there's a lot of annotations and just one line or another is missing the meta-character: the result is that one line is quietly not executed).

Great way of putting it, @pivotaljohn. Thanks for your thoughts!

... sends a bad message to new adopters, who are probably using ytt in part because they can't change the upstream document.

@DanielJonesEB this is why I'm a major advocate of making sure we very clearly document comment behavior up front once it's fully settled. While I was originally in the camp of allowing regular YAML comments by default, they've got a very good logical case for why that shouldn't happen. So, parameterizing this (as an option in the CLI or as a special header) is probably a great way of going about it since that addresses how it's handled for entire IO stream or particular file being processed.

So, 100% on board with POLA. In fact, I think this approach is in line with that, considering the users are now adopting something so new and different like ytt. It's reasonable to expect someone who's adopting it to familiarize themselves with a DSL like ytt's templating language, since it looks like YAML but up front it's clearly quite different. If you combine that clear and up-front documentation with the fact that ytt already already leverages comment syntax to implement imperative logic to otherwise declarative markup (as a core language feature), it should help to dramatically reduce that "astonishment" factor.

pivotaljohn commented 3 years ago

Delighted to report that with this latest release — v0.32.0 ; which includes #324 — this issue has been resolved.

c33s commented 1 year ago

@pivotaljohn just stumbled upon this issue while using the playground. i am completly new to ytt and of course try to simply copy and paste existing yaml files into the playground. throwing errors on valid yaml files really scares away new user, it was a real wtf moment for me.

having a yaml comment in a yaml file which i want to spice up with ytt should simply be kept. ytt is the first template language which behaves like that. for example in puppet you have the epp templating. it would be real stange if i have a comment in my for example nginx config file, which i want to parse with epp and epp would start filtering my comments. in my perspective this is a real odd behavior. in epp you have a epp specific tag to add a comment which will not be in the processed file. <%# COMMENT %> — Removed from the final output https://www.puppet.com/docs/puppet/5.5/lang_template_epp.html none of the engines i use regularly (epp, erb, twig & esh -(just to name a few), filter comments from the template file in the parsing process or even throw an error because of it.

it would be so helpful to allow regular yaml comments (no special handling, just the normal #). maybe add an option like --strict for throwning the error.

the argument that # alone is ambiguous feels a little strange as you could have choosen a different symbol for the ytt language as the #, which is a "core" symbol of yaml. i would vote for "no error on #" and "keep # comments" in the resulting file.

bcdurden commented 1 year ago

Wanted to chime in that this issue still is not resolved. If you are running ytt against a stringified clout-init, the REQUIRED #cloud-config header is removed making it invalid.

patricknelson commented 1 year ago

Check out #63 for the request (read: "ongoing saga") to support emitting comments in the resulting output.

And @c33s try the flag --ignore-unknown-comments to at least skip the errors.

p.s.

having a yaml comment in a yaml file which i want to spice up with ytt should simply be kept

I refer you to the years long discussion above 😅. While I don't use ytt anymore, I'm not sure this'll ever end up being simple or easy to do by default.

carvel-dev / ytt

[lang] Support for standard YAML comments #14

TL;DR.

Details