chrusty / protoc-gen-jsonschema

Protobuf to JSON-Schema compiler
Apache License 2.0
496 stars 101 forks source link

Remove new lines in descriptions. #109

Closed jcchavezs closed 2 years ago

jcchavezs commented 2 years ago

Generated JSON schema descriptions contain new lines chars which are meaningless in a description, caused mainly by the parsed I think. For example in https://github.com/envoyproxy/envoy/blob/9d5627a0879b0a029e90515137c108e1d2884bfc/api/envoy/config/accesslog/v3/accesslog.proto#L33-L35

  // The name of the access log extension to instantiate.
  // The name must match one of the compiled in loggers.
  // See the :ref:`extensions listed in typed_config below <extension_category_envoy.access_loggers>` for the default list of available loggers.
  string name = 1;

generates something like:

"properties": {
    "name": {
        "type": "string",
        "description": "The name of the access log extension to instantiate.\n The name must match one of the compiled in loggers.\n See the :ref:`extensions listed in typed_config below \u003cextension_category_envoy.access_loggers\u003e` for the default list of available loggers."
    },

If not default case I believe it deserves an option.

I am happy to work it out myself.

chrusty commented 2 years ago

Hi @jcchavezs, thanks for raising this. Would you suggest just stripping the \n from description fields?

jcchavezs commented 2 years ago

I think so. My main thought on this is that the single new lines are mainly unintentional, following a max line size rule in proto files. More than one new line is most likely to be user intent.

On Thu, Jan 20, 2022, 21:20 Prawn @.***> wrote:

Hi @jcchavezs https://github.com/jcchavezs, thanks for raising this. Would you suggest just stripping the '\n' from description fields?

— Reply to this email directly, view it on GitHub https://github.com/chrusty/protoc-gen-jsonschema/issues/109#issuecomment-1017895438, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXOYARVOYPWZKHX7DY5V3DUXBVCDANCNFSM5MLHZE4A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

chrusty commented 2 years ago

Actually those newline characters are disgusting. They should totally be stripped.

jcchavezs commented 2 years ago

Do you want me to come up with a PR?

chrusty commented 2 years ago

No it's OK i can handle that

chrusty commented 2 years ago

Hi @jcchavezs, I just merged a fix for this: https://github.com/chrusty/protoc-gen-jsonschema/pull/113

I'll make a release soon

chrusty commented 2 years ago

I've made this change, merged it, and included in a new release: https://github.com/chrusty/protoc-gen-jsonschema/releases/tag/1.3.6

Thanks again for your interest in the project!