Cleanup README.rst: #418

NickVolynkin · 2017-04-28T04:39:29Z

Refactor list items.
Use autonumbering in lists to prevent errors.
Indent code inside list items to make it belong to those items.
Use inline-code style for filenames.
Update link to a pull request with new repo path.
Fix a few typos and punctuation.
Use .. code:: bash for proper highlighting
Use # for comments in bash

polyzen · 2017-04-28T05:46:19Z

@NickVolynkin, this is reStructuredText, not Markdown. Please review the differences between your version and upstream's (specifically the changes to the note and code directives). Looks good to me, otherwise. 👍

NickVolynkin · 2017-04-28T06:40:32Z

Hi, @polyzen!

this is reStructuredText, not Markdown

I'm quite well aware of that. :)

the changes to the note

If you mean that it doesn't render as a Note block – this didn't change. Looks like GitHub just doesn't support this syntax.

and code directives

Suddenly, GH is less forgiving to wrong indentation than Shpinx which I used to check the output. Fixed it in code blocks throughout the document.

NickVolynkin · 2017-04-28T06:41:49Z

The list in "Releasing the Theme" is a step-by-step instruction, so I've changed it to an ordered list.

NickVolynkin · 2017-04-28T06:50:40Z

this is reStructuredText, not Markdown

Oh, the commit message! Yep, that's just a habit. Fixed. :)

polyzen · 2017-04-28T06:58:50Z

Looks like GitHub just doesn't support this syntax.

That's a shame. The docs don't specify a blank line here -- probably best to inline it.

NickVolynkin · 2017-04-28T07:08:28Z

Most directives in this files have a blank line between declaration and content. But if the docs say so, let it be without a blank line. )

* Refactor list items. * Use autonumbering in lists to prevent errors. * Indent code inside list items to make it belong to those items. * Use inline-code style for filenames. * Update link to a pull request with new repo path. * Fix a few typos and punctuation. * Use .. code:: bash for proper highlighting * Use # for comments in bash * Use ordered list for step-by-step instuctions.

NickVolynkin · 2017-04-28T07:13:04Z

inline it

Done.

agjohnson

The content changes here look 👍 . My only nitpick to these edits is that across our documentation we do use 4 space indentation, as 3 spaces is not a requirement of the reST spec, and it's better our indentation matches our python source.

NickVolynkin · 2017-04-29T11:22:57Z

@agjohnson it seems to me that reST indeed requires indent equal to that of the directive. Look what happens if I use 4 and 8 space indents in the list items: https://github.com/NickVolynkin/sphinx_rtd_theme/tree/four-space-indent#set-up-your-environment.

If there's a way to use "normal" 4 space indentation and keep the code formatting, please tell me. It's something that I struggle a lot with.

NickVolynkin · 2017-04-29T11:27:57Z

I'm quite new to reST and now things seem like this to me. Am I wrong?

#. In an ordered list text starts from the fourth character.

   So I should indent next lines with 3 spaces to make them belong to the list.

* In a bulleted list text starts from the third character.

  So next lines should be indented with 2 spaces.

#. Real hell starts

   * When you have a sub-list

     Now it's 3+2 spaces.

     .. code:: bash

        # three more spaces, 8 total.

polyzen · 2017-04-30T03:36:10Z

"List item bodies must be left-aligned and indented relative to the bullet .."

4-space indentation must be for directive contents (possibly just the code directives?).

Blendify · 2017-05-09T02:10:49Z

For one of my projects, we use 3 space indentation. I do not have a strong opinion but I think 3 is correct. @agjohnson can we merge this?

agjohnson · 2017-05-09T16:53:50Z

3 space indentation isn't wrong, as directives just require >3 spaces. We use 4 everwhere else though.

agjohnson · 2017-05-09T17:07:31Z

@NickVolynkin Your example is mostly correct, and @polyzen describes the pattern we use -- list items require special indentation and we use 4 space indentation on all directives. I can comment on the specific blocks in review.

In particular, for a code block nested in a list:

1. Items can be single lines
2. Items can be multiple,
   dangling lines, but indentation must match (3 chars)
3. Items can contain

   spaces in between lines as long as indentation matches,
   this includes directives:

   .. code::

       # This is 7 character indentation
       # 3 for the list item plus 4 for code
       print(sys.version)

agjohnson

Here's an example of some of the indentation changes for what our normal style would be.

agjohnson · 2017-05-09T17:07:58Z

README.rst

-    html_theme = "sphinx_rtd_theme"
-    html_theme_path = ["_themes", ]
+   html_theme = "sphinx_rtd_theme"
+   html_theme_path = ["_themes", ]


This should be 4 space indent

agjohnson · 2017-05-09T17:08:06Z

README.rst

+       'collapse_navigation': False,
+       'display_version': False,
+       'navigation_depth': 3,
+   }


4 space indent here

agjohnson · 2017-05-09T17:09:21Z

README.rst


-    pip install sphinx
+      pip install sphinx


This code block would be 7 space indent (3 for list item, 4 for code)

agjohnson · 2017-05-09T17:09:37Z

README.rst


-    gem install sass
+      gem install sass


7 space indent here

agjohnson · 2017-05-09T17:09:45Z

README.rst

-    // Now that everything is installed, let's install the theme dependecies.
-    npm install
+      # Now that everything is installed, let's install the theme dependencies.
+      npm install


7 space indent here

NickVolynkin · 2017-05-10T02:33:59Z

@agjohnson thanks for explaining. Will update the MR next week. :)

Cleanup README.rst * Refactor list items. * Use autonumbering in lists to prevent errors. * Indent code inside list items to make it belong to those items. * Use inline-code style for filenames. * Update link to a pull request with new repo path. * Fix a few typos and punctuation. * Use .. code:: bash for proper highlighting * Use # for comments in bash * Use ordered list for step-by-step instuctions. * Make all code blocks 4 space indents

Blendify · 2017-06-17T18:45:11Z

This pull request became inactive and #418 fixes the indent issue so that was merged instead. Nick Volynkin, please still free to send more PRs in the future.

* commit '6c2302472fb2c61eec9b4be3a2515117fffbfac3': (253 commits) Add pygments_style to theme.conf Document new theme option Add option to layout.html Update theme.conf Apply clearfix on breadcrumbs Follow-up grunt for readthedocs#472 (readthedocs#476) Only change current if a matching link is found (readthedocs#472) Add a couple of badge icons to readme (readthedocs#471) Add Sphinx Framework for PyPi (readthedocs#470) Fix error in setup entry points (readthedocs#466) Added `.admonition` to the CSS (readthedocs#462) Use explicit UTF8 encoding (readthedocs#452) Add missing pygments css link Convert underscores to dashes. Reflect language in lang attribute. Replace master with v0.2.5 in README.rst Add missing dependency in dev setup: sphinxcontrib-httpdomain (readthedocs#431) Finalized readthedocs#418 (readthedocs#433) Added a link to the Sphinx documentation about setuptools entry points Added setuptools entry point for Sphinx ...

agjohnson added the PR: ready for review label Apr 28, 2017

NickVolynkin force-pushed the cleanup-readme branch from 03399e5 to 53c0ca9 Compare April 28, 2017 04:42

NickVolynkin force-pushed the cleanup-readme branch from 53c0ca9 to 89ecc0c Compare April 28, 2017 06:37

NickVolynkin force-pushed the cleanup-readme branch from 89ecc0c to 8e7ca66 Compare April 28, 2017 06:43

NickVolynkin changed the title ~~Cleanup README.MD:~~ Cleanup README.rst: Apr 28, 2017

NickVolynkin force-pushed the cleanup-readme branch from 8e7ca66 to 86719bf Compare April 28, 2017 06:50

NickVolynkin force-pushed the cleanup-readme branch from 86719bf to f776424 Compare April 28, 2017 07:07

polyzen approved these changes Apr 28, 2017

View reviewed changes

NickVolynkin force-pushed the cleanup-readme branch from f776424 to a1c6887 Compare April 28, 2017 07:12

polyzen approved these changes Apr 28, 2017

View reviewed changes

agjohnson requested changes Apr 28, 2017

View reviewed changes

agjohnson requested changes May 9, 2017

View reviewed changes

benjaoming mentioned this pull request Jun 16, 2017

Finalized #418 #433

Merged

Blendify closed this Jun 17, 2017

agjohnson removed the PR: ready for review label Jun 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup README.rst: #418

Cleanup README.rst: #418

NickVolynkin commented Apr 28, 2017 •

edited

Loading

polyzen commented Apr 28, 2017 •

edited

Loading

NickVolynkin commented Apr 28, 2017

NickVolynkin commented Apr 28, 2017 •

edited

Loading

NickVolynkin commented Apr 28, 2017

polyzen commented Apr 28, 2017 •

edited

Loading

NickVolynkin commented Apr 28, 2017

NickVolynkin commented Apr 28, 2017

agjohnson left a comment

NickVolynkin commented Apr 29, 2017

NickVolynkin commented Apr 29, 2017

polyzen commented Apr 30, 2017 •

edited

Loading

Blendify commented May 9, 2017

agjohnson commented May 9, 2017

agjohnson commented May 9, 2017

agjohnson left a comment

agjohnson May 9, 2017

agjohnson May 9, 2017

agjohnson May 9, 2017

agjohnson May 9, 2017

agjohnson May 9, 2017

NickVolynkin commented May 10, 2017

Blendify commented Jun 17, 2017

Cleanup README.rst: #418

Cleanup README.rst: #418

Conversation

NickVolynkin commented Apr 28, 2017 • edited Loading

polyzen commented Apr 28, 2017 • edited Loading

NickVolynkin commented Apr 28, 2017

NickVolynkin commented Apr 28, 2017 • edited Loading

NickVolynkin commented Apr 28, 2017

polyzen commented Apr 28, 2017 • edited Loading

NickVolynkin commented Apr 28, 2017

NickVolynkin commented Apr 28, 2017

agjohnson left a comment

Choose a reason for hiding this comment

NickVolynkin commented Apr 29, 2017

NickVolynkin commented Apr 29, 2017

polyzen commented Apr 30, 2017 • edited Loading

Blendify commented May 9, 2017

agjohnson commented May 9, 2017

agjohnson commented May 9, 2017

agjohnson left a comment

Choose a reason for hiding this comment

agjohnson May 9, 2017

Choose a reason for hiding this comment

agjohnson May 9, 2017

Choose a reason for hiding this comment

agjohnson May 9, 2017

Choose a reason for hiding this comment

agjohnson May 9, 2017

Choose a reason for hiding this comment

agjohnson May 9, 2017

Choose a reason for hiding this comment

NickVolynkin commented May 10, 2017

Blendify commented Jun 17, 2017

NickVolynkin commented Apr 28, 2017 •

edited

Loading

polyzen commented Apr 28, 2017 •

edited

Loading

NickVolynkin commented Apr 28, 2017 •

edited

Loading

polyzen commented Apr 28, 2017 •

edited

Loading

polyzen commented Apr 30, 2017 •

edited

Loading