helm-values-schema-json icon indicating copy to clipboard operation
helm-values-schema-json copied to clipboard

Add support for multiline comments above property

Open Silthus opened this issue 1 year ago • 6 comments

Regarding #51 it would also be very helpful to allow comments above a property to be parsed for the schema tags.

For example:

# My description of this really cool property.
@ @schema title:"My Property"
# @schema maxLength:10;pattern:^[a-z]+$
my_property: some value

The above also defaults a non annotated comment above a property to use as the description. Maybe this could be turned on with a flag in the cli?

Silthus avatar Apr 11 '24 12:04 Silthus

Top comment is a bit of a problem because of helm-docs. We would need to fight for the same space and eventually something won't work as expected. That's the reason why I decided for line comment which makes it impossible to be multi line. However, I'm open for ideas, suggestions or pull request.

losisin avatar Apr 11 '24 12:04 losisin

Top comment is a bit of a problem because of helm-docs. We would need to fight for the same space and eventually something won't work as expected. That's the reason why I decided for line comment which makes it impossible to be multi line. However, I'm open for ideas, suggestions or pull request.

And what about being compatible with helm-docs so that the same comments can be used for schema generation and helm-docs? This would even make a lot of sense, since as a Chart Maintainer I would want to generate a schema for validation and tooling support in IDEs and a Readme that contains the descriptions and types as well.

Silthus avatar Apr 11 '24 12:04 Silthus

I was referring more to development of this plugin. When do you start reading comments and when do you stop? Is there something before or after in comments that should be ignored? Is it with annotation for schema, helm-docs or regular user comment? I avoided all of that by reading line comment. I understand that having all those things is very convenient for end users but moving to top comment will also introduce breaking change for existing users. For the record, I'm not against it but can't promise anything short term. Maybe for future major release like v2. Until then, I'll leave this issue open so that people can vote 👍 👎

losisin avatar Apr 11 '24 13:04 losisin

My 2 cents:

  • After having used both helm-docs and a combination of this tool and Adobe's jsonschema2md tool, I like the docs I get from jsonschema2md better. It's also a more generic solution, it works with any JSON schema, not just helm charts.
  • What about making it optional? like helm schema blah blah --above-line-mode (I hate it lol, there's definitely a better name for it out there somewhere). If the optional flag is enabled, the tool doesn't try to fight with helm-docs, it just assumes nothing else is there and it gets to do whatever it wants.

Side node: I'm loving this tool, thanks a bunch @losisin for putting it out there, it's been perfect for my use case. If you turn on your sponsor button I want to buy you a beer or two

RothAndrew avatar Jun 05 '24 18:06 RothAndrew

Different idea: if the # @schema comment is on a different line, require a field that ties it back to the yaml key that you want it associated with. This would open things up to let you put it anywhere you want.

image:
  repository: registry.example.com/foo
  tag: 1.2.3

# @schema ref: image; description: Image settings
# @schema ref: image.repository; description: The repository to use; required: true; default: registry.example.com/foo
# @schema ref: image.tag; description: The tag to use

A side benefit would be I can do things this way and make the values.yaml file more readable for downstream users. It takes a lot of brain power to grok the yaml among all the line comments.

This example doesn't demonstrate the original ask of this issue though, as it still does just one line for each. Not sure how multiline might look, maybe something like

# @schema ref: image;
# description: Some long comment that spans
# multiple lines;
# @endschema

IDK, I'm sure there's probably a better way out there.

RothAndrew avatar Jun 05 '24 22:06 RothAndrew

I like the idea of adding a ref and the @endschema and the benefits it brings :)

Silthus avatar Nov 15 '24 05:11 Silthus

If we want comments above property specifically, and want to not collide with helm-docs, then the # @schema lines must come before the description:

# @schema maxLength:10;pattern:^[a-z]+$
# -- My description of this really cool property.
my_property: some value

This is because helm-docs reads the comment line by line, and once it finds the "start description token" (e.g # -- or # @raw --) then it will include all the following lines, unless those lines are also some other helm-docs specific lines like # @default -- or # @section --.

Helm-docs does not have a way to define "this is where the description ends", except for when the comment block ends.

Ref: https://github.com/norwoodj/helm-docs/blob/f5b3ede7b103eec1ac7f89e5ac37b5fbc5a795ce/pkg/helm/comment.go#L11

We would probably also want to borrow helm-docs' logic for only considering comments directly above the property, and ignore (or preferably error) when there's a gap like this:

# @schema title: this is ignored, errors, or possibly means "add title to schema root"

# @schema default: this works fine
# -- This sets the pull policy for images.
pullPolicy: IfNotPresent

This does not exclude the possibility of adding schema comments somewhere else as a way to document it. Though I would suggest a slightly different syntax:

image:
  repository: registry.example.com/foo
  tag: 1.2.3

# @schema.image description: Image settings
# @schema.image.repository description: The repository to use; required: true; default: registry.example.com/foo
# @schema.image.tag description: The tag to use

I think with this the feature could be enabled by default. However if you want helm-docs' # -- comment to be parsed by helm-values-schema-json then you would opt-in via something like a --with-helm-docs=true flag.

applejag avatar May 29 '25 13:05 applejag