Page MenuHomePhabricator

Diffusion - add ToC for readme files
ClosedPublic

Authored by btrahan on Mar 11 2014, 10:01 PM.
Tags
None
Referenced Files
F14345137: D8490.id20153.diff
Thu, Dec 19, 1:05 AM
Unknown Object (File)
Mon, Dec 16, 1:23 AM
Unknown Object (File)
Tue, Dec 10, 2:59 PM
Unknown Object (File)
Thu, Dec 5, 10:08 AM
Unknown Object (File)
Wed, Dec 4, 10:00 PM
Unknown Object (File)
Wed, Dec 4, 10:00 PM
Unknown Object (File)
Wed, Dec 4, 10:00 PM
Unknown Object (File)
Wed, Dec 4, 9:51 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).