Diff highlighting broken (looks streaming search related)

[rvantonder]

Looks like a regression with streaming search, because it seems to manifest only with streaming on:

example:

With streaming off:

Commit message highlighting via type:commit appears unaffected. Not sure if backend or frontend so just assigning everyone.

[github-actions]

Heads up @lguychard - the "team/search" label was applied to this issue.

[keegancsmith]

Will take a look. I believe this happens without streaming as well sometimes. Need to see if I can find the other issue about this (or maybe I just have it in my org files).

[keegancsmith]

This is correctly highlighted in the src-cli + streaming, which makes me think it has something to do with how webapp highlights commits. I think this is an old issue, I do have memory of seeing it before:

src search -stream 'repo:^github\.com/sourcegraph/sourcegraph$ type:diff return count:10'

vs

(match count difference is due to using a lower count: limit when I did the cli query)

[limitedmage]

Looking. Also ran into the separate issue: https://sourcegraph.slack.com/archives/C07KZF47K/p1616608611137800 (update: that issue is now resolved)

[limitedmage]

Root cause: The library we're using to convert Markdown to HTML, Marked, converts tabs to spaces with no way to disable this. So all our highlighting calculations are broken once the tabs have been converted to spaces.

This doesn't happen in non-streaming search because we return HTML of the code instead of Markdown.

[limitedmage]

Not sure what the best way to fix this is. @keegancsmith What is the feasibility of modifying streaming search backend to return HTML like GraphQL search does instead of Markdown?

[keegancsmith]

Not sure what the best way to fix this is. @keegancsmith What is the feasibility of modifying streaming search backend to return HTML like GraphQL search does instead of Markdown?

We can do that. It is a bit strange we have markdown as the response type, really you would just expect a raw byte buffer like we get with line content/etc. So I'd like to better understand how this all works. I would expect we can have a better arch for streaming before we start having 3rd party clients depend on the API. It seems we ask the backend to highlight code blocks, I see requests to graphql asking to highlight a codeblock adn we get back HTML. Is there a better way to structure this overall?

[stefanhengl]

@keegancsmith and I discussed this a bit. We think the backend should just send back content as plain text, without the ```COMMIT_MSG or ```diff markers. The frontend wouldn't have to call "Marked" at all. We would still call the backend for highlighting though.

Only somewhat related, we noticed that sending content back to the backend for highlighting can take a lot of time. Maybe streaming could send down the first N events already highlighted? @limitedmage WDYT?

[limitedmage]

I think we should move forward with returning HTML identical to the GraphQL response as we know this works. We can work on optimizing later (eg. returning HTML already with syntax highlighting).