alex-oleshkevich / starception

Beautiful exception page for Starlette apps.
MIT License
96 stars 6 forks source link

Syntax highlighting in the browser #17

Closed jtraub closed 1 year ago

jtraub commented 1 year ago

What if we do syntax highlighting in the browser? Starception already uses js to switch between frames so moving syntax highlighting to the client side is something that can be considered.

I am evaluating Highlight.js for this. A custom build can be created that contains just two themes and highlighters for selected languages (python, js, html, (s)css) and then integrated into Starception.

This would allow us to drop pygments dependency and use prefers-color-scheme media feature to automatically switch between light and dark themes. Lack of auto switching depending on day/night is the most irritating thing for me right now.

What are your thoughts @alex-oleshkevich ? Would you be willing to accept a PR with such change?

alex-oleshkevich commented 1 year ago

Yes, I agree. That was the long term plan (see README) but I was quite satisfied with the current state so didn't care much of it. If you wish to contribute the feature, I would be happy to merge it.

jtraub commented 1 year ago

@alex-oleshkevich sorry for bothering again but what is your js target? Do you target only evergreen browsers?

The reason I am asking this is following. In #18 I purposefully didn't use closest as it is supported in 96.7% of tracked desktop browsers and instead went with data attributes / dataset property which has 100% desktop support.

Now I am seeing that in recent commits you've used closest so thing brings my question. If we target modern browsers then it is fine - the highlight-related code can be made shorted and simpler.

alex-oleshkevich commented 1 year ago

Yes, only modern browsers. That's an internal tool, so support of near edge browsers is reasonable.

On Wed, Oct 19, 2022, 18:06 Konstantin Mikhailov @.***> wrote:

@alex-oleshkevich https://github.com/alex-oleshkevich sorry for bothering again but what is your js target? Do you target only evergreen browsers?

The reason I am asking this is following. In #18 https://github.com/alex-oleshkevich/starception/pull/18 I purposefully didn't use closest as it is supported in 96.7% of tracked desktop browsers and instead went with data attributes / dataset property which has 100% desktop support.

Now I am seeing that in recent commits you've used closest so thing brings this question. If we target modern browsers then it is fine - the highlight-related code can be made shorted and simpler.

— Reply to this email directly, view it on GitHub https://github.com/alex-oleshkevich/starception/issues/17#issuecomment-1284164455, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE3HSBLC72IO3TTIMLI253WEAFATANCNFSM6AAAAAARILDD6M . You are receiving this because you were mentioned.Message ID: @.***>

alex-oleshkevich commented 1 year ago

I would like to defer any bundlers as late as possible and stick to vanilla JS embedding it into HTML response body.

alex-oleshkevich commented 1 year ago

@jtraub i have an idea how we (possibly) can reach it without the refactoring. What if we switch pygments to use classes instead of inline styles and embed two stylesheets into the HTML wrapped into media query? This will switch colors using your system settings.

jtraub commented 1 year ago

@alex-oleshkevich I was thinking about that before and there was something that stopped me from switching pygments back to classes. Let me check again in the morning.

If it would work I will simply do a PR instead.

alex-oleshkevich commented 1 year ago

@jtraub works perfectly. Need more testing and polishing before merge. https://github.com/alex-oleshkevich/starception/pull/22

alex-oleshkevich commented 1 year ago

Merged.