Page MenuHomePhabricator

Diffusion - add ToC for readme files
ClosedPublic

Authored by btrahan on Mar 11 2014, 10:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jan 18, 5:29 AM
Unknown Object (File)
Wed, Jan 15, 7:52 AM
Unknown Object (File)
Tue, Jan 14, 5:21 AM
Unknown Object (File)
Tue, Jan 7, 5:04 PM
Unknown Object (File)
Mon, Dec 30, 1:16 AM
Unknown Object (File)
Fri, Dec 27, 2:35 PM
Unknown Object (File)
Tue, Dec 24, 4:37 AM
Unknown Object (File)
Sat, Dec 21, 2:41 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).