Page MenuHomePhabricator

Diffusion - add ToC for readme files
ClosedPublic

Authored by btrahan on Mar 11 2014, 10:01 PM.
Tags
None
Referenced Files
F15456993: D8490.id20139.diff
Sun, Mar 30, 1:31 PM
F15451100: D8490.id.diff
Fri, Mar 28, 8:08 PM
F15429174: D8490.id20153.diff
Mon, Mar 24, 1:01 AM
F15410276: D8490.id20153.diff
Wed, Mar 19, 6:59 AM
F15382576: D8490.diff
Fri, Mar 14, 1:14 PM
F15354874: D8490.id20153.diff
Tue, Mar 11, 5:38 AM
F15354416: D8490.diff
Tue, Mar 11, 3:54 AM
F15334863: D8490.id20153.diff
Sat, Mar 8, 12:44 PM

Details

Summary

see title. Fixes T4549.

Test Plan

made a readme that had some headers and observed a nice ToC

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

btrahan retitled this revision from to Diffusion - add ToC for readme files.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added reviewers: epriestley, chad.
epriestley edited edge metadata.

This implementation looks correct to me, although I'm not 100% convinced the feature is valuable on the balance. If we suppose most READMEs are small and have, like, 2-3 sections (and the actual docs are, like, not in a giant README file) then I could see this getting in the way more than it helps. Your call, though; we can also see if anyone complains, or we could activate the TOC only at some threshold (like at least 5 sections).

This revision is now accepted and ready to land.Mar 11 2014, 10:29 PM

I sort of assumed it would take 3 sections to trigger.

It currently requires 2 to trigger (in all cases where we render a ToC).

Let's see what people say / do not say. :) I think this is going to work out quite well as I think READMEs tend to be very sparse (ToC won't render) or very dense (ToC is valuable).

btrahan updated this revision to Diff 20153.

Closed by commit rP740757fd9b7a (authored by @btrahan).