Page MenuHomePhabricator

HTML email doesn't syntax highlight code blocks, inline diffs, or diff context above inline comments
Closed, ResolvedPublic

Description

Comparing the email I received regarding D14481#162798 with the same comment as viewed in Differential:

EmailDifferential

It seems that we aren't syntax highlighting HTML emails.

Event Timeline

joshuaspence raised the priority of this task from to Needs Triage.
joshuaspence updated the task description. (Show Details)
joshuaspence added a project: Mail.
joshuaspence added a subscriber: joshuaspence.
epriestley triaged this task as Wishlist priority.Nov 24 2015, 5:22 PM
epriestley added a project: Remarkup.
scode added a subscriber: scode.Mar 29 2016, 11:57 PM
epriestley renamed this task from HTML email doesn't syntax highlight code blocks to HTML email doesn't syntax highlight code blocks, inline diffs, or diff context above inline comments.Mar 30 2016, 12:09 AM
epriestley added a subscriber: epriestley.

See also T5701 for bonus complexity.

eadler added a project: Restricted Project.Mar 30 2016, 5:45 AM
eadler moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Mar 30 2016, 5:48 AM
epriestley moved this task from Backlog to The Queue on the Prioritized board.

I think this isn't really too bad. The major issue is that we can't use stylesheets in email because Gmail doesn't support them, so we need to emit markup with inline style attributes.

pygmentize (and other first-party highlighters) spits out tags and I think that's going to have to be our input, but the input should be correctly formed and the stakes for mis-parsing it are low (you can't normally XSS users in email).

All the pygmentize rules are single-class CSS rules (e.g., always .syntax .thing, never .syntax .context .thing) so we don't need to parse it in some proper stack-based way. In fact, we can probably use a regular expression.

We do need a runtime-accessible mapping from CSS classes to color rules, and making these maps swappable enables T5701. I'm probably going to vaguely prepare for that but not specifically try to tackle it yet, although maybe I'll try to swap to a single definition and generate syntax.css from it since I don't really want to have two copies of the color rules.

So we're probably looking at:

  • write a parser for Pygmentize markup;
  • extract all the color rules from syntax.css and write something that regenerates it from the authority;
  • use the color map + Pygmentize parser to highlight code for email.
f($v, "v"); /* ! */
file.c
f($v, "v"); /* ! */
counterexample.c
f($v, "v"); /* ! */
f($v, "v"); /* ! */

That seems to be doing something fairly reasonable in my client, yell if it looks funky somewhere else:

This doesn't extend to inline diffs yet, just code blocks inside existing rendered remarkup blocks.