Comparing the email I received regarding D14481#162798 with the same comment as viewed in Differential:
Differential | |
It seems that we aren't syntax highlighting HTML emails.
joshuaspence | |
Nov 15 2015, 12:12 AM |
F1258966: Screen Shot 2016-05-05 at 2.57.02 AM.png | |
May 5 2016, 10:00 AM |
F968225: html.png | |
Nov 15 2015, 12:12 AM |
F968223: email.png | |
Nov 15 2015, 12:12 AM |
Comparing the email I received regarding D14481#162798 with the same comment as viewed in Differential:
Differential | |
It seems that we aren't syntax highlighting HTML emails.
Status | Assigned | Task | ||
---|---|---|---|---|
Resolved | epriestley | T992 Send rich HTML mail bodies without impacting reply handling functionality | ||
Resolved | epriestley | T10694 Improve behavior of context for inline comments in mail | ||
Resolved | None | T9790 HTML email doesn't syntax highlight code blocks, inline diffs, or diff context above inline comments |
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:
f($v, "v"); /* ! */
f($v, "v"); /* ! */
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.