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

[codemirror] Bump version to 5.6.0, also package addons #199

Merged
merged 1 commit into from
Aug 24, 2015
Merged

[codemirror] Bump version to 5.6.0, also package addons #199

merged 1 commit into from
Aug 24, 2015

Conversation

plexus
Copy link
Contributor

@plexus plexus commented Aug 23, 2015

CodeMirror comes with a large list of addons that can be opted in to. In
the previous version these weren't being packaged, now they are.

@plexus
[codemirror] Bump version to 5.6.0, also package addons
2fb7898 
CodeMirror comes with a large list of addons that can be opted in to. In
the previous version these weren't being packaged, now they are.
plexus added a commit to plexus/QuilFiddle that referenced this pull request Aug 23, 2015
@plexus
Bump to latests Cljsjs package for codemirror, use addons from there
00b066d 
As soon as this is released cljsjs/packages#199
it's no longer needed to have the codemirror addons copied into this
repo. We can just pull them in from the package.

Bind Ctrl-Enter to eval-sexp for non-mac users.
crisptrutski added a commit that referenced this pull request Aug 24, 2015
@crisptrutski
Merge pull request #199 from plexus/codemirror_package_addons
6e05e40 
[codemirror] Bump version to 5.6.0, also package addons
@crisptrutski crisptrutski merged commit 6e05e40 into cljsjs:master Aug 24, 2015
@plexus
Copy link
Contributor Author

plexus commented Aug 24, 2015

Thanks! @martinklepsch would you mind reviewing in any case, since you wrote the original?

@martinklepsch
Copy link
Member

I don't think I did, it was @Deraen I believe.
Also I guess Chris reviewed as well.

Did you encounter problems or is something not working?
On Mon 24 Aug 2015 at 10:36 Arne Brasseur notifications@github.com wrote:

Thanks! @martinklepsch https://github.com/martinklepsch would you mind
reviewing in any case, since you wrote the original?

—
Reply to this email directly or view it on GitHub
#199 (comment).

@Deraen
Copy link
Member

Deraen commented Aug 24, 2015

The code looks good to me.

@plexus
Copy link
Contributor Author

plexus commented Aug 24, 2015

It took a few tries but it's working now. The only thing I'm not sure of is if it needs extra extern definitions for the addons. I'll try out a few with advanced optimizations tonight.

@crisptrutski
Copy link
Member

Hey Arne 👋 😄

Sure noting the externs affected by http://codemirror.net/doc/releases.html would be great

Regarding current externs, I'm not sure where CodeMirrorObj and CodeMirrorCursor come in (they're not available when requiring the main package). Are these definitions still required?

Was wondering whether it made more sense to put add-ons into their own packages, but a quick test with advanced compilation shows the DCO is working well (only 3kb added) - yay!

@martinklepsch martinklepsch mentioned this pull request Aug 30, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@plexus @martinklepsch @Deraen @crisptrutski