Skip to content

Added CommonMark support to all description #1049

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

Merged
merged 2 commits into from
Apr 26, 2017

Conversation

webron
Copy link
Member

@webron webron commented Apr 23, 2017

Fixes #879

@webron webron requested a review from darrelmiller April 23, 2017 01:03
versions/3.0.md Outdated
@@ -324,7 +324,7 @@ An object representing a Server.
Field Name | Type | Description
---|:---:|---
<a name="serverUrl"></a>url | `string` | A URL to the target host. This URL supports Server Variables and may be relative, to indicate that the host location is relative to the location where the OpenAPI definition is being served. Variable substitutions will be made when a variable is named in `{`brackets`}`.
<a name="serverDescription"></a>description | `string` | An optional string describing the host designated by the URL.
<a name="serverDescription"></a>description | `string` | An optional string describing the host designated by the URL. [CommonMark syntax](http://spec.commonmark.org/) can be used for rich text representation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies CommonMark is optional, but it is not.

It is more correct to say, "Description strings are CommonMark text." here and in the remaining changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to keep it as-is for now. Will submit a separate PR to remove the word 'optional' from a lot of property descriptions instead as there's no need to mention that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is ambiguous and can/should be simplified (more concise, not ambiguous). I'm not referring to the use of the word "optional", but the phrase "can be used", which implies something other than CommonMark can be used if the author does not want rich text. This is CommonMark.

@whitlockjc
Copy link
Member

My worry is that "Markdown" is a well-known language and calling it "CommonMark" is confusing to me. If it's not to others, this should not stop us from shipping it.

@ePaul
Copy link
Contributor

ePaul commented Apr 26, 2017

Maybe "This string is Markdown in its CommonMark variant." would be better?

@MikeRalphson
Copy link
Member

MikeRalphson commented Apr 26, 2017

Bikeshedding "This string SHOULD be interpreted as Markdown according to the CommonMark specification."?

@webron
Copy link
Member Author

webron commented Apr 26, 2017

This PR was meant to make sure CommonMark can be used in all the descriptions, not to tackle the wording of the description field. We can tackle that separately.

@webron webron merged commit af42644 into OpenAPI.next Apr 26, 2017
@webron webron deleted the commonmark-descriptions branch April 26, 2017 18:40
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.

6 participants