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

Integrate Proselist #2049

Open
ericholscher opened this issue Mar 8, 2016 · 6 comments
Open

Integrate Proselist #2049

ericholscher opened this issue Mar 8, 2016 · 6 comments
Labels
Accepted Accepted issue on our roadmap Feature New feature

Comments

@ericholscher
Copy link
Member

https://github.com/amperser/proselint is a neat tool. It would be cool to run it and display results on the build pages, or fail builds on failure levels, etc.

@ericholscher ericholscher added the Improvement Minor improvement to code label Mar 8, 2016
@agjohnson agjohnson added the Accepted Accepted issue on our roadmap label Sep 18, 2018
@agjohnson
Copy link
Contributor

Also, another option is a sphinx extension, so that all of this happens inside Sphinx and it's own validation and warning system. I started down this path a while ago but didn't see a great amount of value in the basic regex sort of validation proselint provides.

More useful to authoring standards, but much harder to quantify in a meaningful way, are metrics like:

  • Is this copy too complex?
  • Is this copy on topic to the page?
  • Is this copy extraneous to the topic?
  • Are the images/UI referenced by this copy up to date?
  • Does copy match UI it's referencing?

Leaving this as a maybe feature for RTD, but I think the implementation point is outside RTD.

@humitos
Copy link
Member

humitos commented Jul 18, 2019

It would be cool to run it and display results on the build pages, or fail builds on failure levels, etc.

I'm not sure if we want to go in this direction. It sounds more like a CI feature than a Read the Docs one to me.

I think it could be cool if we can integrate it in a way that differentiate us from just running running the tool from the command line. I'm imagining this as something that can be ran on Pull Requests and highlight sentences in-doc where problems are found by proselint, and maybe a tooltip with the error message on it.

This way, while reviewing the documentation of the PR, the reviewer will immediately see where the problem is, read the suggestion and propose it back in the review.

Leaving this as a maybe feature for RTD, but I think the implementation point is outside RTD.

All that I mentioned before could be done by a Sphinx extension, but it would need to be "enabled by default" on Pull Requests, or at least support a config option for this.

@davidfischer
Copy link
Contributor

another option is a sphinx extension

I think a Sphinx/MkDocs extension seems like the right approach.

@pranay414
Copy link
Contributor

Can I work on this?

@humitos
Copy link
Member

humitos commented Jul 22, 2019

@pranay414 sure! I suggest you to start writing a design document about how do you plan to implement this. I'm re-labeling this as Feature since it seems we want to go into a Sphinx/MkDocs extension direction.

You can get some inspiration for the design doc from https://www.freecodecamp.org/news/how-to-write-a-good-software-design-document-66fcf019569c/

@humitos humitos added Feature New feature and removed Improvement Minor improvement to code labels Jul 22, 2019
@humitos
Copy link
Member

humitos commented Aug 25, 2019

It's possible to easily implement this by replacing the source text from the reStructuredText file with a :abbr: for example (this could be easily replaced with a better tooltip, though).

import proselint

def setup(app):
    app.connect('source-read', source_read)

def source_read(app, docname, source):
    rawsource = source[0]
    # check, message, line, column, start, end, extent, severity, replacement
    suggestions = proselint.tools.lint(rawsource)

    for check, message, line, column, start, end, extent, severity, replacement in reversed(suggestions):
        content = rawsource[start-1:end].strip()
        injected = f' :abbr:`{content} ({message})` '
        rawsource = rawsource[:start-1] + injected + rawsource[end:]

    source[0] = rawsource

Although, this easy implementation has some drawbacks/limitations since it does not understand reStructuredText and the replacement could be done inside a directive or a role that we don't want.

This is an example of the output generated by this small extension:

screenshot--2019 08 24-23_25_36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature
Projects
None yet
Development

No branches or pull requests

5 participants