Page MenuHomePhabricator

Diffusion - add ToC for readme files
ClosedPublic

Authored by btrahan on Mar 11 2014, 10:01 PM.
Tags
None
Referenced Files
F13192234: D8490.diff
Sun, May 12, 6:24 AM
F13181859: D8490.id20153.diff
Thu, May 9, 4:18 PM
F13181856: D8490.id20139.diff
Thu, May 9, 4:18 PM
F13181826: D8490.id.diff
Thu, May 9, 4:05 PM
F13181801: D8490.diff
Thu, May 9, 3:53 PM
Unknown Object (File)
Mon, May 6, 2:04 AM
Unknown Object (File)
Wed, Apr 24, 10:32 PM
Unknown Object (File)
Wed, Apr 24, 4:28 AM

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