I think this is a good idea. One concern I have is: does this mean Tracker would support these nicer diffs but other apps rendering markdown wouldn't? is it handled per-app right now? I think we should make sure that we keep a "Sourcegraph Style Markdown" for example, that remains consistent across all apps.
I agree with that.
This is not specific to Tracker, it is specific to using a specific Markdown renderer (namely, this one).
We certainly can and should expand that to all of Sourcegraph, not just apps.
I used that Markdown renderer in Tracker because I was building this platform app at a later time and had the freedom of picking a Markdown renderer myself. The core Sourcegraph codebase uses another less sophisticated Markdown renderer (a blackfriday.Html
renderer with a certain set of options set). It previously used at least 2 different Markdown renderers in the frontend (written in JS).
The end goal future direction is to have a Sourcegraph-specific blackfriday.Renderer
IMO. This current existing renderer can be a good starting point. But we need to figure out and agree on the specifics separately.
The end goal future direction is to have a Sourcegraph-specific blackfriday.Renderer IMO. This current existing renderer can be a good starting point. But we need to figure out and agree on the specifics separately.
I think this is a very good idea. How about we start migrating in this direction right now, rather than later? We don't have to migrate all uses over to it just yet -- but we could start out by just declaring a "Sourcegraph markdown" in a Go package somewhere -- and making use of it in Tracker to begin with? This way we could continue making improvements to it as we need more changes like this one. What do you think?
Agreed.
How about src.sourcegraph.com/markdown
as the import path?
It can have a single method func Render(markdown []byte) (sanitizedHTML []byte)
or similar:
// Render renders and sanitizes markdown.
func Render(markdown []byte) (sanitizedHTML []byte) {
return github_flavored_markdown.Markdown(markdown)
}
That could be the initial implementation; we can make it more custom and elaborate (i.e., bring in actual source of github_flavored_markdown
package and be able to make changes to it) as our needs increase and time budget allows.
Agreed. We'll need to add support for some custom markdown syntax (@ mentions, emoji, etc.) eventually, so would be a good idea to start making this possible now.
The original issue has been resolved, and diffs are rendered nicely now. It uses the relevant CSS from Primer.
It was done in https://github.com/shurcooL/issuesapp/commit/3c28bdba09fcf83b7cc784c7428d750b6b98f903.
The rendered markdown already marks up
diff
fenced code blocks with highlighting (including intra-block character-level highlighting). However, Sourcegraph CSS, which is being used by Tracker, does not include any implementation of those classes.So a
diff
fenced code block currently appears plain. E.g.:If we add the CSS for those classes, which are "gi", "gd", "gu", "gh" for outer blocks, "x" for inner emphasis blocks (source), then the above will appear in a more readable highlighted form:
/cc @slimsag @renfredxh