CodeMirror 3 #2944

Closed
wants to merge 4 commits into from

5 participants

@Carreau
IPython member

Totally remove codemirror from source tree and assume it is a component.

You HAVE TO install codemirror in /static/component/ by cd into /notebook/ and issue a $ bower install (it will install bootstrap, less and codemirror-head in the right place)

We need to find a way to separate dev-dependencies from running dependencis.

The rest of the PR are fixes to work with new CM version.
Only missing thing is ? as a single operator, which is not correctly highlighted.

Of course this is only for test purpose, see how hard it was.

Bugs :

  • cursor position after line wrapping.
  • md cell -> md highlight
  • tooltip, increase z index (see through codemirror)
@takluyver
IPython member
@jasongrout
IPython member

Does this handle version dependencies? Will it install exactly the right version of codemirror for the currently-checked-out git tree? Can I update the codemirror installation if I check out a different version?

@Carreau
IPython member

Hang on, does this mean anyone who wants to run from git will need extra
set up steps? I agree about separating external dependencies, but it's a
useful feature that people can very easily test development versions.

No, this is just a test to see how things are going with CM3. Of course we will keep dev usable as-is.
Probably with a build step that bundle the right files in the right place.

Does this handle version dependencies? Will it install exactly the right version of codemirror for the currently-checked out git tree? Can I update the codemirror installation if I check out a different version?

Yes, bower does deal with dependencies, right now it target "latest" but one can say in the right file to target a particular range of version number or even a specific sha.

Right now this is mostly to see how easily we can use a "stock" version of codemirror that does not have extra patches, and test bower.

I would say that you can checkout any version of CM3 as long as it is under /component/codemirror directory it should work, so you can also use submodules or stuff like that.

@Carreau
IPython member

Python mode for codemirror is now configurable, we can use the singleOperator config option to match ? instead of maintaining a patched version.

We could also maybe probably config for % and %% magics.

@ellisonbg
IPython member

This is a great start.

  • You have another PR that moves the ipython CM css theme (#2942). Should that one be closed?
  • At the dev meeting, we talked about having our bower components at the top level of static, so it would be static/codemirror, etc. This will mean adding a .browerrc file one level up that sets the directory option to static.
  • Code mirror doesn't use proper git tags that bower can recognize, so it will be difficult for us to depend on a particular version of codemirror. We need to think about how we want to handle this.
  • Obviously, we do want to ship a stable CM in static/codemirror.
  • I am about to start the "great JavaScript code reorganization" so we need to finish this PR up soon.
  • Can you update your list of things that need to get done for this to be finished? (matching ?)
@Carreau
IPython member

You have another PR that moves the ipython CM css theme (#2942). Should that one be closed?

I would prefere #2942 to be merged,
this one is mainly an experiment to test the transition (not too hard)

Code mirror doesn't use proper git tags that bower can recognize

Doesn't bower understand commit hash ?

@ellisonbg
IPython member

There are some bugs in our version of CM that I think CM 3 might fix. I would like to move to CM 3, but maybe this is not the PR for that? Should we close this? Originally, you said that #2942 should be merged instead? What do you think? I would love to get our CM stuff cleaned up as it is blocking some other work.

@Carreau
IPython member

There are some bugs in our version of CM that I think CM 3 might fix. I would like to move to CM 3, but maybe this is not the PR for that?

Need to be rebased cleaned up and tested. Their are probably bugs in usage.
Question is, if we do it this week, I won't be there to fix things the 2 weeks after.

Should we close this?

I'll rebase soon.

Originally, you said that #2942 should be merged instead?
Merged before. it was to ease this one.

I would love to get our CM stuff cleaned up as it is blocking some other work.
Should I commit all CM fils or leave it as a component ?
In a component folder or elsewhere ?

@Carreau Carreau was assigned Apr 9, 2013
Carreau added some commits Feb 16, 2013
@Carreau
IPython member

I rebase and updated a little.

Could someone install codemirror through bower, it is not going through the F-@$%&-G proxy anymore, so I had to test by git cloning manually.

Then, could the same person issue a PR agains my branch and/or open a new one ?

Also this need to commit files in the component directory (once codemirror installed), wherease not all of files in component directory are required (bootstrap...). So this will conflict with one of min's recent PR.

One of the things that we could do is .gitignore specific subpath of components.

@jasongrout
IPython member

@williamstein mentioned to me that CM3 has some performance issues with resizing many instances that CM2 (possibly) didn't have. Maybe it would be good to test a page with many CM3 instances with content, and resize the page (i.e., resize the instances). I haven't done the test myself, but it would be good to check.

@williamstein
@williamstein
@ellisonbg
IPython member

Closing, this PR has been replaced by #3232

@ellisonbg ellisonbg closed this Apr 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment