Page MenuHomePhabricator

Rough cut of inline comment context
ClosedPublic

Authored by epriestley on May 5 2016, 11:57 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 9:17 PM
Unknown Object (File)
Fri, Dec 20, 3:59 PM
Unknown Object (File)
Fri, Dec 20, 8:23 AM
Unknown Object (File)
Tue, Dec 17, 11:21 PM
Unknown Object (File)
Thu, Dec 12, 3:00 AM
Unknown Object (File)
Sun, Dec 8, 7:32 PM
Unknown Object (File)
Wed, Dec 4, 10:24 AM
Unknown Object (File)
Tue, Dec 3, 4:32 AM
Subscribers
None

Details

Summary

Ref T10694. This is still missing some pieces, but seems to get most of the data into the mail in a plausible format:

  • When an inline remarks on code, show the patch inline in the mail body.
  • When an inline replies to another inline, show that other inline in the mail body.
  • Apply remarkup rendering to inline content.
  • Apply basic styling to mail body blocks.

Not covered yet:

  • Syntax highlighting.
  • Diff highlighting.
  • Maybe clearer style/layout hints to connect comments to what they reply to? Current approach might get messy with inlines that have blockquotes and code blocks inside them, for example.
  • I probably want to cap the amount of diff context we ever show to ~7 lines, even if you drag over 200 lines of code.
  • CSS is a generally a bit rough still.
  • The unified-comment-context option is effectively always on now, and should be removed.
  • Text section is getting indented right now but probably shouldn't be.
  • Spacing, etc., might be a bit off.
Test Plan

Rigged Home to render these things, got a plausible-looking render (top is text, bottom is HTML):

Screen Shot 2016-05-05 at 4.50.18 AM.png (494×329 px, 32 KB)

Sent myself some inline comment mail, got a plausible result.

Diff Detail

Repository
rP Phabricator
Branch
inline1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 12062
Build 15193: Run Core Tests
Build 15192: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Rough cut of inline comment context.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.
This revision is now accepted and ready to land.May 5 2016, 4:17 PM
This revision was automatically updated to reflect the committed changes.
src/applications/differential/mail/DifferentialInlineCommentMailView.php
380

test test test

380

test? test!

380
test