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

substitute '@@' code block delimiter with triple backtick, more con… #5513

Merged
merged 3 commits into from
Oct 20, 2017

Conversation

nastasi-oq
Copy link
Contributor

…sistent with other markdown extensions

following the discussion on the previous pull request (#5462) I moved from:

@@ <lang> @@
...
block of code
...
@@

to:

``` <lang>
...
block of code
...
\```

(that extra backslash is just so I can fool githubs markdown parsing to let me put triple backticks inside a code block marked out by triple backticks)

Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Looks good in principle. Just the test that’s failing.

@nastasi-oq
Copy link
Contributor Author

nastasi-oq commented Oct 19, 2017

I'm in the same releases matrix of https://travis-ci.org/encode/django-rest-framework/jobs/290016056 and it works on my machine.
Do you have some suggestion ?

@carltongibson
Copy link
Collaborator

Hey @nastasi-oq see gem#1 (comment)

I refactored the test a bit, to show the rendered markdown.

E     <p><code>json
E     [{
E         "alpha": 1,
E         "beta: "this is a string"
E     }]</code></p>

It looks like it's leaving the json declaration unprocessed.

@nastasi-oq
Copy link
Contributor Author

nastasi-oq commented Oct 20, 2017

@carltongibson the point is that triple backtick is managed in any case (without pygments) as block of <code>, fixed.
If you want to have a full coverage you must add pygments and markdown optional installation as 2 new dimensions of tox matrix.
(I was testing with pygments but travis has not).
Currently Pygments is not yet in the optional requierements too.

@carltongibson
Copy link
Collaborator

I was testing with pygments but travis has not

Fine. That explains that.

@carltongibson carltongibson added this to the 3.7.2 milestone Oct 20, 2017
Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

OK. Lets have this.

Thanks @nastasi-oq!

@carltongibson carltongibson merged commit 9ec81e3 into encode:master Oct 20, 2017
@merwok
Copy link
Contributor

merwok commented Nov 7, 2017

Hello! I had missed that one could use markdown in view docstrings to get nice code blocks, since it’s more common to have reST in the Python world, so first let me say thanks!

This PR seems nice, but I can’t make it work locally. After installing markdown and pygments I tried json and javascript as lexer name, with and without leading space, with and without a blank line following. (I know reST and Sphinx very well but always forget details of Github Markdown.)

Could the docs give a simple example?

@merwok
Copy link
Contributor

merwok commented Nov 7, 2017

FTR: Looking at the source code, it seems like the pygments_css function is only used by the DocumentationRenderer, which only works with include_docs_url. The classic HTML API renderer has markdown code blocks without colors.

@carltongibson
Copy link
Collaborator

... it seems like the pygments_css function is only used by the DocumentationRenderer...

@merwok This is because it's only just been added. A PR back porting this to the Browsable API would be welcome I guess.

@merwok
Copy link
Contributor

merwok commented Nov 7, 2017

That’s interesting! I am not ready to switch to include_docs_url in my projects, so I would like to improve Browsable API. I’ll open a ticket if one isn’t already.

@merwok
Copy link
Contributor

merwok commented Nov 9, 2017

Quick update: it turns out that both Documentation and Browsable renderers are quite different but both end up calling apply_markdown. The generated HTML for BrowsableAPIRenderer already contains the pygments markup, only the CSS is missing.

@merwok
Copy link
Contributor

merwok commented Nov 9, 2017

PR coming up tomorrow!

screenshot

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