OpenDataServices / sphinxcontrib-opendataservices

Sphinx directives maintained by Open Data Services
MIT License
0 stars 0 forks source link

Parse text for the `jsoninclude-quote` directive as markdown #41

Closed Bjwebb closed 1 year ago

Bjwebb commented 1 year ago

Example: https://sphinxcontrib-opendataservices.readthedocs.io/en/jsoninclude-quote-markdown/jsoninclude/#json-include-quoted

odscjames commented 1 year ago

Is it technically a backwards incompatible change, and should this be behind some kind of Markdown allowed flag? Are we certain this won't break existing use?

When that's clear, actual code looks great and happy to approve.

(Generally speaking, I've also been pushing the idea that we don't want JSON Schema files to be caring to much about the context of the standard docs, because someone else might take the JSON schema files and use them elsewhere. I don't think this means we shouldn't do this, but it would be interesting to have a conversation about what is appropriate or not to use. eg links to external sites like "www.externaldocs.com" is fine but internal links like "/other-page-in-our-docs.html" should be used very careful because that may end up in a different context where it is a broken link.)

Bjwebb commented 1 year ago

Good question.

This directive was only added a couple of weeks ago. AFAIK, only the open fibre docs are using it, which is where the request for the change has come from.

Also, it behaves the same if you don't include any markdown syntax.

But, equally it would be fine to put this behind some kind of flag.

odscjames commented 1 year ago

Also, it behaves the same if you don't include any markdown syntax.

Yes, but if you include some markdown syntax and expect it to be rendered as text you are in for a surprise.

Also, are we sure that's true - might not different new line combinations change the output?

This directive was only added a couple of weeks ago.

In that case, it seems fine to do either then - up to you.

Bjwebb commented 1 year ago

Yes, but if you include some markdown syntax and expect it to be rendered as text you are in for a surprise.

Yes, this is true.

Also, are we sure that's true - might not different new line combinations change the output?

Yes, I suspect newlines were ignored entirely before.

In that case, it seems fine to do either then - up to you.

I think I'm still happy to merge as is. 0.x revisions can have breaking changes, and I don't think it will actually adversely affect anyone.