Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Docstrings highlighting with pygments#5462

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
carltongibson merged 5 commits intoencode:masterfromgem:docstrings-with-pygments2
Oct 2, 2017

Conversation

@nastasi-oq
Copy link
Contributor

Currently:

  • tests are missing

Copy link
Collaborator

@carltongibsoncarltongibson left a comment

Choose a reason for hiding this comment

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

👀s good.

@nastasi-oq
Copy link
ContributorAuthor

nastasi-oq commentedSep 30, 2017
edited
Loading

QUESTION@carltongibson: looking to adding test I found thatapply_markdown(text): function (https://github.com/encode/django-rest-framework/blob/master/rest_framework/compat.py#L233)
has more or less the same behavior ofrender_markdown(markdown_text) function (https://github.com/encode/django-rest-framework/blob/master/rest_framework/templatetags/rest_framework.py#L70) but the first has tests and a better convertion in case markdown package is not present.
There is any particular reason becauserender_markdown isn't a simple wrapper ofapply_markdown ?

@carltongibson
Copy link
Collaborator

@nastasi-oq I think “accident of history” is probably the simplest answer.

I’m happy to see a refactoring here but it might be better to handle it in two steps. This PR adding the new functionality, not worrying about duplication. Then another PR handling the refactoring. This makes each step easier to review, and complete. Sound ok to you?

@nastasi-oq
Copy link
ContributorAuthor

nastasi-oq commentedOct 1, 2017
edited
Loading

@carltongibson IMHO there are 2 options:

  1. do what you suggest but in the reverse order
  2. do all in a single PR
    The reason is that extend the current test for apply_markdown is far simple and fast than develop a new one that in the second PR will be removed.
    If I have enough time before release I can do 1.

@nastasi-oq
Copy link
ContributorAuthor

@carltongibson new PR opened to refactorrender_markdown:#5469

@nastasi-oqnastasi-oq changed the titleWIP: Docstrings highlighting with pygmentsDocstrings highlighting with pygmentsOct 2, 2017
@nastasi-oq
Copy link
ContributorAuthor

@carltongibson for me is final.

Copy link
Collaborator

@carltongibsoncarltongibson left a comment

Choose a reason for hiding this comment

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

This is lovely. Nice small footprint. Thanks for the work!

@carltongibsoncarltongibson merged commit063534a intoencode:masterOct 2, 2017
@nastasi-oq
Copy link
ContributorAuthor

I'm glad to contribute to DRF.

carltongibson reacted with heart emoji

@dmwyatt
Copy link
Contributor

@nastasi-oq Where did this@@ method of marking code blocks come from? Is that a standard somehwere or just something implemented here?

I ask because it's pretty widespread to mark code blocks in markdown with triple backticks. For example, that's how you mark code blocks here on github.

@nastasi-oq
Copy link
ContributorAuthor

@dmwyatt just from the example that I found (and that is credited in the code), if you think to be better change it in another syntax I'm not against.

@dmwyatt
Copy link
Contributor

@carltongibson What do you think?

To be clear on what I'm talking about, this PR uses a format that is not used in any other markup system that I can find. Specifically, to mark a code block with this PR you do something like:

@@ json @@['some json here']@@

This '@@' markup is what I'm talking about.

Here on github and lots of other places you use triple backticks to markup code blocks. Of course, I don't think the Markdown "standard" (such as it is) hasany method of marking code blocks, but I think triple backticks are widely-supported.

I propose code blocks be marked like:

```json['some json here']\```

(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)

@carltongibson
Copy link
Collaborator

Yep. I’d happily review a PR making an adjustment here. I’m not particularly worried about the block markers, but the triple back ticks are widely used.

(In a way it would be nice to allow the markdown processor to be pluggable. I suspect that’s not going to be a hook we’d want to expose.)

@nastasi-oq
Copy link
ContributorAuthor

nastasi-oq commentedOct 19, 2017
edited
Loading

@carltongibson always happy to make people happy: review it 😜 !

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@carltongibsoncarltongibsoncarltongibson approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

3.7.0 Release

Development

Successfully merging this pull request may close these issues.

3 participants

@nastasi-oq@carltongibson@dmwyatt

[8]ページ先頭

©2009-2025 Movatter.jp