md_browser does not render nested lists correctly |
|||||
Issue descriptionWhat steps will reproduce the problem? (1) tools/md_browser/md_browser.py -d docs (2) wget http://localhost:8080/android_studio.md -O out.html (3) % grep 'The wizard' out.html What is the expected result? Nested lists are rendered correctly. What happens instead? Nested lists are not rendered correctly. Please use labels and text to provide additional information. android_studio.md has nested lists like this one: * Avoid running the setup wizard. * The wizard will force you to download unwanted SDK components to `//third_party/android_tools`. * To skip it, select "Cancel" when it comes up. In Gitiles, nested lists are rendered correctly: https://chromium.googlesource.com/chromium/src/+/master/docs/android_studio.md In md_browser, second level items are enclosed in <pre><code>...</code></pre>: <ul> <li>Avoid running the setup wizard.<pre><code>* The wizard will force you to download unwanted SDK components to `//third_party/android_tools`. * To skip it, select "Cancel" when it comes up. </code></pre> </li> </ul>
,
Jul 5 2017
I was hit by this too. I'm not using gitiles to preview markdown, which is pretty painful.
,
Jul 22 2017
Mike, your https://codereview.chromium.org/2772173002 changed the markdown.Markdown() call in src/tools/md_browser/md_browser.py to pass tab_length=2. Per discussion on https://github.com/waylan/Python-Markdown/issues/3, this argument determines the amount of indenting needed for nested lists. When I pass 4 (the default) instead, nested lists using 4-space indent are interpreted correctly. Google's Markdown style guide says to use 4-space indent for nested lists: https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#nested-list-spacing. This is what we use throughout Chrome and Chrome OS repositories. The Gitiles doc referenced by your change (https://gerrit.googlesource.com/gitiles/+/master/Documentation/markdown.md#Lists) also uses 4 (and then 5!)-space indent for nested lists, but then goes on to use 2-space indent for paragraphs embedded in a single-level list: --- 1. Fruit In botany, a fruit is the seed-bearing structure in flowering plants. In common language usage, "fruit" normally means the fleshy seed-associated structures of a plant that are sweet or sour, and edible in the raw state, such as apples, bananas, grapes, lemons, oranges, and strawberries. 2. Cake The cake is a lie. --- This diverges from the "spec" at https://daringfireball.net/projects/markdown/syntax#list: "List items may consist of multiple paragraphs. Each subsequent paragraph in a list item must be indented by either 4 spaces or one tab". As I understand it, md_browser is intended to be used to preview files that will later be rendered by Gitiles, so it should match Gitiles's behavior as closely as possible. I don't think that md_browser can match it exactly, because I don't think there's any way to get Python-Markdown to use Gitiles's weird "at least 2 leading spaces" logic for embedded paragraphs. But!! Gitiles's docs appear to be wrong: when I use the exact text from their example, the paragraph isn't embedded (gitiles_docs_incorrect.png). So my thinking is that we should switch md_browser to use tab_length=4 and then continue telling people to write their docs like this: --- * List item Here's an embedded paragraph. It can be more than one line. * Nested list item More nested text. --- This lines up intuitively, and follows the Google style guide, and is rendered the same in md_browser (with tab_length=4) and Gitiles, as far as I can tell. I'm attaching some more screenshots that may help illustrate the differences.
,
Jul 22 2017
Grepping through *.md in the chromium/src repo, there are examples of people using basically any style, because hey, Markdown. :-/ I see:
* Here's a multi-line
bullet point.
and:
* Here's a multi-line
bullet point.
and even:
* Here's a multi-line
bullet point.
https://chromium.googlesource.com/chromium/src/+/master/docs/README.md says that docs must follow the Google style guide. My reading of https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#lists is that 4-space ident should be used everywhere, with an exception carved off for trivial single-line, non-nested lists:
"When nesting lists, use a 4 space indent for both numbered and bulleted lists ... Even when there's no nesting, using the 4 space indent makes layout consistent for wrapped text ... However, when lists are small, not nested, and a single line, one space can suffice for both kinds of lists."
My preference is still to make md_browser use 4-space indent, since it'll steer people toward doing what they should probably be doing in any case.
,
Sep 2 2017
I'm not sure when I'll get to this; marking it available in case someone else wants to look into what to do here ...
,
Sep 2 2017
Per #4, does anyone object to me making md_browser go back to using 4-space indents?
,
Sep 5 2017
The approval for this is deafening, so going ahead.
,
Oct 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/db0868eb17e4f5a0ca928a14024f6ee8c061c0e8 commit db0868eb17e4f5a0ca928a14024f6ee8c061c0e8 Author: Daniel Erat <derat@chromium.org> Date: Fri Oct 20 05:30:39 2017 md_browser: Switch to 4-space indent. The Google Markdown style guide advocates 4-space indent[1]. When configured to use 2-space indent (what we were using before), md_browser doesn't render 4-space-indented nested lists or wrapped text correctly. Gitiles (used by Gerrit and chromium.googlesource.com to render Markdown) also wants 4-space indent rather than 2-space indent. 1. https://github.com/google/styleguide/blob/gh-pages/docguide/style.md#nested-list-spacing Bug: 736199 Change-Id: Icf4aa298f893672b6a7718aa1dc3e300f6559eff Reviewed-on: https://chromium-review.googlesource.com/729749 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Dan Erat <derat@chromium.org> Cr-Commit-Position: refs/heads/master@{#510349} [modify] https://crrev.com/db0868eb17e4f5a0ca928a14024f6ee8c061c0e8/tools/md_browser/md_browser.py
,
Oct 20 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by satorux@chromium.org
, Jun 23 2017