Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/playground: add code highlighting #28539

Closed
cn007b opened this issue Nov 1, 2018 · 6 comments
Closed

x/playground: add code highlighting #28539

cn007b opened this issue Nov 1, 2018 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@cn007b
Copy link
Contributor

cn007b commented Nov 1, 2018

Is it possible to improve "Go Playground" code highlight?
And instead of black code have something like default github code highlight,
for example something like this.

@mvdan
Copy link
Member

mvdan commented Nov 1, 2018

Duplicate of #21123, which was already a duplicate of a golang-nuts thread. I don't think anything has changed since.

@mvdan mvdan closed this as completed Nov 1, 2018
@ALTree
Copy link
Member

ALTree commented Nov 2, 2018

@mvdan Note that at the end of that golang-nuts thread, Andew Gerrand wrote:

For the record: Christoph Hack said he was going to prepare a change
to restore the more sophisticated editor to the playground. This would
include a syntax highlighting option that defaults to "off" (as per
the tour). If this change can be made without any regressions then it
will be accepted.

This was back in 2012, but it appears that adding syntax colouring to the playground wasn't completely out of the question (in fact, Andrew explicitly said the change would be accepted). It is also still true that tour has a "Syntax on/off" button; so it wouldn't be that strange to add it to the playground.

The other linked issue (#21123) wasn't really resolved with a final decision, the OP just closed it because they were able to find a playground clone with syntax highlighting.

I think it would be nice to have a final and clear decision on this; for this reason I'm reopening this issue.

cc @andybons

@ALTree ALTree reopened this Nov 2, 2018
@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Nov 2, 2018
@ALTree ALTree changed the title Go Playground - code highlight x/playground: add code highlighting Nov 2, 2018
@gopherbot gopherbot added this to the Unreleased milestone Nov 2, 2018
@mvdan
Copy link
Member

mvdan commented Nov 2, 2018

I hadn't noticed that; reopening for now is indeed a good idea.

@ysmolski
Copy link
Member

ysmolski commented Nov 2, 2018

Here we decided that syncing playground with a CodeMirror library (the one used by go-tour) is a no-go: #18723. Andy suggested to move the tour to simpler library instead of adding sophisticated lib to the playground for the sake of syntax highlighting.

My opinion echoes Andy. It should be solved by adding custom code that does what is required only, no bloat and slow rendering. It should not be done by including big libraries where we would use 1% of functionality.

@andybons
Copy link
Member

andybons commented Nov 2, 2018

Agreed with @ysmolsky. As we converge the codebases, we don't want to regress syntax highlighting on the tour, so it should remain supported across the two tools. As long as it's lazy-loaded and not on by default I don't see an issue.

Merging into #18723 as this will be included.

@andybons andybons added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Nov 2, 2018
@andybons
Copy link
Member

andybons commented Nov 2, 2018

Duplicate of #18723

@andybons andybons marked this as a duplicate of #18723 Nov 2, 2018
@andybons andybons closed this as completed Nov 2, 2018
@golang golang locked and limited conversation to collaborators Nov 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants