search

esprehn / chromium-codereview

A chrome extension that changes the frontend to Rietveld for Chromium
Other
35 stars 11 forks source link

Multiline string syntax highlighting broken #112

Open aarongable opened 9 years ago

aarongable commented 9 years ago

See screenshots.

In the first screenshot, the beginning of the multiline string is not in the diff (although it is in the context snippet), but the end it. This appears to result in the actual string not being highlighted (it is black), but the next piece of context (a class declaration) being colored green like a string.

In the second screenshot, context has been shown. The newly revealed context is correctly highlighted, but it didn't fix the incorrect highlighting of the original diff.

screenshot from 2014-11-05 10 43 09

screenshot from 2014-11-05 10 39 44

esprehn commented 9 years ago

This is really hard to fix because the syntax highlighting is done on the client, and we often don't have the context to know what state to start highlighting in. Thankfully this is pretty rare.

aarongable commented 9 years ago

It's not rare in Python at all, and a significant portion of codereview on Rietveld is in Python. I hit this in what feels like over half of my reviews.

Can the client-side syntax highlighting be re-run when context is loaded? That would resolve the majority of the cases.

esprehn commented 9 years ago

Yeah I can resyntax highlight when context is loaded. Note that another way to fix this would be to expose the base files from the server, or the hash/rev of the base file, in an API so I can run the highlighter to get the right mode. I'd really prefer that, right now context expansion is a huge hack.