Page MenuHomePhabricator

Diffusion - re-jigger how README files get rendered
ClosedPublic

Authored by btrahan on Aug 22 2014, 9:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 11, 11:53 PM
Unknown Object (File)
Sun, Dec 8, 5:57 PM
Unknown Object (File)
Sun, Dec 8, 8:45 AM
Unknown Object (File)
Sun, Dec 8, 4:54 AM
Unknown Object (File)
Wed, Dec 4, 7:21 AM
Unknown Object (File)
Tue, Dec 3, 8:32 PM
Unknown Object (File)
Sat, Nov 30, 2:41 PM
Unknown Object (File)
Nov 14 2024, 2:09 PM

Details

Summary

be more aggressive about assuming plain-text, use remarkup for no extension, .remarkup, and .md, and last but not least use rainbow for .rainbow. Fixes T5818.

Test Plan

my README rendered just fine post these changes

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

btrahan retitled this revision from to Diffusion - re-jigger how README files get rendered.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.
This revision is now accepted and ready to land.Aug 22 2014, 9:56 PM
epriestley edited edge metadata.

Oh, hmm...

src/applications/diffusion/conduit/DiffusionReadmeQueryConduitAPIMethod.php
80–84

Oh -- do we get this wrong?

That is, if we have README.rst and also README.remarkup, is it possible we'd render one with the wrong rendering type?

This revision now requires changes to proceed.Aug 22 2014, 9:57 PM
src/applications/diffusion/conduit/DiffusionReadmeQueryConduitAPIMethod.php
81–83

That is, I'd expect we need something like:

$best_render_type = $render_type;

...here, and then to use $best_render_type below.

btrahan edited edge metadata.

nice catch, fixed

epriestley edited edge metadata.
This revision is now accepted and ready to land.Aug 22 2014, 10:09 PM
asherkin added inline comments.
src/applications/diffusion/conduit/DiffusionReadmeQueryConduitAPIMethod.php
49

This change doesn't seem correct? Files without an extension are generally going to be plain text - and appear to conflict with "be more aggressive about assuming plain-text".

Rendering actual plain text as remarkup is fine. It basically just gets links linked up, which is good.

It's only a problem when the text is formatted as something else and then we try to render it as remarkup and everything gets all terrible.

I'd be fine with rendering "README" (i.e., no extension) as plain text too, but not having links clickable seems slightly worse than what we generate from plaintext documents I've seen. If I'm wrong and some reasonable subset of no-extension files are actually terrible when rendered as remarkup we can/should change the rule to assume ".txt" from "README".

I'd be fine with rendering "README" (i.e., no extension) as plain text too, but not having links clickable seems slightly worse than what we generate from plaintext documents I've seen. If I'm wrong and some reasonable subset of no-extension files are actually terrible when rendered as remarkup we can/should change the rule to assume ".txt" from "README".

Na, doing it to linkify them makes complete sense.

btrahan updated this revision to Diff 24894.

Closed by commit rPc1e8d97069fa (authored by @btrahan).