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

Don't remove newlines from OpenAPI descriptions #370

Merged
merged 2 commits into from
Mar 24, 2023
Merged

Don't remove newlines from OpenAPI descriptions #370

merged 2 commits into from
Mar 24, 2023

Conversation

silas
Copy link
Contributor

@silas silas commented Nov 16, 2022

Update protoc-gen-openapi to not remove newlines from descriptions as this breaks markdown formatting.

@silas silas requested a review from a team as a code owner November 16, 2022 06:37
@timburks
Copy link
Contributor

Hi @silas, thanks for noting this! YAML has options for configuring how newlines are treated in strings (reference). The files affected by your change used "literal" style, where newlines were kept. We could consider instead writing strings in "folded" style, where newlines in the YAML file are replaced with spaces. Do you think that would fix the Markdown problems that you're observing? (I'm concerned that single-line strings could be very long and make the generated specs hard to read)

@silas
Copy link
Contributor Author

silas commented Nov 30, 2022

Hi @silas, thanks for noting this! YAML has options for configuring how newlines are treated in strings (reference). The files affected by your change used "literal" style, where newlines were kept. We could consider instead writing strings in "folded" style, where newlines in the YAML file are replaced with spaces. Do you think that would fix the Markdown problems that you're observing? (I'm concerned that single-line strings could be very long and make the generated specs hard to read)

I think the "folded" style is still going to break markdown.

I'm not 100% sure I understand the single-line strings problem. Right now gnostic removes all the newlines, which effectively creates a long single-line string, which is more difficult to read (at least to me) in raw YAML form and via tooling.

Is there a specific tool you use which doesn't render the new style well? I'm happy to take a look.

You can try the below code example via the Swagger Editor to see how different styles render (or see the screenshots below).

openapi: 3.0.3
info:
    title: Example
    version: 0.0.1
paths:
    /example:
        get:
            description: Some example.
            parameters:
                - name: example
                  in: query
                  description: |-
                    This is an example description.
                    
                    We want some interesting content here.
                    
                    Some examples:
                      - Example 1
                      - Example 2
                      - Example 3
                      
                    ## Code

                        Some block of code.
                  schema:
                    type: string
            responses:
                "200":
                    description: OK

Current PR (literal)

literal

Current PR (folded)

folded

Main branch (newlines replaced with strings)

current

@morphar
Copy link
Collaborator

morphar commented Dec 10, 2022

@silas I think this looks good and agree that it's much more readable.
And very nice, not breaking Markdown :)

@timburks could you elaborate on your concerns? I don't think I quite understood either.
Is the issue that there will be more lines in the YAML file?
I can see how long descriptions could add up to the total amount of lines.

I guess it's a question of balancing readability of the "code" of the YAML files and the readability of the output in tools like Swagger editor.

Would a better solution be to not remove the newlines (or replace with \n), but keep the less readable single line definition?

It doesn't seem to break any conversions between formats, unless there's something we haven't thought of.

@timburks timburks merged commit 8c84f3c into google:main Mar 24, 2023
@silas silas deleted the openapi-description branch March 24, 2023 15:52
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