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.

Sign up for GitHub

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

Jump to bottom

x/playground: synchronize editor library with tour #18723

Closed
spf13 opened this issue Jan 19, 2017 · 8 comments
Closed

x/playground: synchronize editor library with tour #18723

spf13 opened this issue Jan 19, 2017 · 8 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Unreleased

Comments

@spf13
Copy link
Contributor

spf13 commented Jan 19, 2017

for unknown reasons the playground and the go tour use completely separate editor libraries. The tour one is much more advanced leveraging http://codemirror.net.

Ideally we would use a single script to power both and simply embed one in the other. To as much of a degree as possible sharing code between them would result in better features and less maintenance.

Somewhat dependent on golang/tour#146
@broady broady added this to the Unreleased milestone Jan 19, 2017
@dmitshur dmitshur self-assigned this Apr 20, 2017
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/41207 mentions this issue.

gopherbot pushed a commit to golang/tour that referenced this issue Apr 25, 2017
@dmitshur @broady
gotour: update CodeMirror editor to 5.25.2
cbbf25e 
Update CodeMirror editor from version 3.16 to the latest current
version, which is 5.25.2.

The goal is to reduce maintenance burden in the future, and
make it easier to enable features like bracket matching, etc.

The update was done by overwriting existing files with those
from the latest CodeMirror release. No new files were added.
I tested gotour locally and the new editor appears to work ok.
There are no errors in console, typing works, and I noticed
at least one issue resolved in the new version. Typing
CodeMirror.version in browser console now prints "5.25.2".

Commit 768e12f applied manual changes
to mode/go/go.js, but these have been merged upstream via
codemirror/codemirror5@047afd2,
codemirror/codemirror5@524f54a, and
codemirror/codemirror5@1ec2263.
As a result, I was able to use mode/go/go.js from upstream
without cherry-picking those changes.

Helps #146.
Helps golang/go#18723.

Change-Id: I94bd687609b80de7a4ed89cf6a300a878ac9bfe6
Reviewed-on: https://go-review.googlesource.com/41207
Reviewed-by: Chris Broadfoot <cbro@golang.org>
@broady broady added the DevExp anything around developer experience label May 31, 2017
@dlsniper
Copy link
Contributor

Hi, maybe considering merging https://goplay.space/ into the official playground would be a good improvement for all gophers? Thank you.

@spf13
Copy link
Contributor Author

spf13 commented Jun 14, 2017 via email

@dmitshur dmitshur removed their assignment May 1, 2018
@ysmolski
Copy link
Member

As cool as goplay.space is, it has diverted a lot from the playground code. I don't think we can "merge" it.

@ysmolski ysmolski changed the title playground: synchronize editor library with tour x/playground: synchronize editor library with tour May 12, 2018
@ysmolski ysmolski removed the DevExp anything around developer experience label Oct 29, 2018
@ysmolski
Copy link
Member

I think this requires rethinking. IMHO adding CodeMirror to Playground means more maintenance. And code in playground right now is simple and requires no updates about CodeMirror lib. So my vote is a no.

@ysmolski ysmolski added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 29, 2018
@ysmolski
Copy link
Member

@andybons please see the comment above.

@andybons
Copy link
Member

CodeMirror is an enormous library and we wouldn’t use 99% of the features. I’d rather move the tour to a simpler editor implementation.

@ysmolski
Copy link
Member

Okay. Adding the whole library just for the sake of syntax highlighting is a no go. Closing.

@ysmolski ysmolski mentioned this issue Nov 2, 2018
Closed
@ALTree ALTree mentioned this issue Sep 11, 2019
Open
@golang golang locked and limited conversation to collaborators Oct 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
7 participants
@broady @ysmolski @spf13 @andybons @dlsniper @dmitshur @gopherbot