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

Update to CodeMirror 3 and start to ship our components #3232

Merged
merged 26 commits into from Apr 29, 2013

Conversation

ellisonbg
Copy link
Member

The main purpose of this branch is the update to CodeMirror 3 and ship it as a bower component. Because CodeMirror doesn't use proper semantic version tags, we have created a CodeMirror ipython fork. The sole purpose of this fork is for us to maintain properly named tags that bower can install from.

As a side effect of these changes, we are now shipping in place all bower components that we are using (bootstrap, less. codemirror, jquery). BUT, the version of jquery in components is not yet being used as we are still stuck with the old version attached to the nasty jquery.ui branch we ship. In a later PR, we will need to get rid of that jquery.ui branch (replace those capabilties with bootstrap) and then move to a stable version of jquery.ui that matches that of the jquery component depended upon by bootstrap.

The transition to CodeMirror 3 is a bit of a pain - lots of things broke. In particular the input cell styling, tooltip and completer broke. I think I have fixed it all, but we need to go over these things with a pixel-perfect eye - alignments of all input/output text - subtle padding/margin changes, etc.

This PR replaces PR #2944

This commit adds the right versions of:

* Bootstrap
* Jquery
* less.js
* CodeMirror

We should always use bower to manage these packages in the
future. BUT, we are not yet using this version of jquery as
we still rely on an older crazy-branch version of jquery.ui.
CM3 introduced a number of changes to how various paddings are set.
Because of how we change the line-height we had to set these back
to the CM2 values. What a pain!
@ellisonbg ellisonbg mentioned this pull request Apr 28, 2013
3 tasks
@ellisonbg
Copy link
Member Author

I should note that the move to CM3 fixed some bugs that were preventing us from fixing the dreaded focus-click bug. Yeah!

def components():
"""install components with bower"""
with lcd(static_dir):
local('bower install')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove the components command? Won't we still want to use this when we update the versions we use?

@minrk
Copy link
Member

minrk commented Apr 28, 2013

Since the diff is completely impenetrable and filled with irrelevant (to us) removals and additions, I guess the only way to 'review' this is to test it in actual use. Actual code review does not seem possible.

@epifanio
Copy link
Contributor

i don't know if my problem is related to this, but after i merged the CM3 branch i'm having trouble to get the "custom js buttons" in the toolbar. this are my files :

this are my files https://gist.github.com/epifanio/5477637

  • reverting to master i got the custom js back

@ellisonbg
Copy link
Member Author

I agree this is a tough one to review. But I think thee are a few points that deserve a check-off:

  • As of this PR, we are shipping all of our bower components (bootstrap, jquery, less, codemirror) in our repo.
  • Someone other than me should run the notebook and put codemirror through its paces to try and find other breakage points. I realize only so much can be done, but we should at least do a little before the merge.
  • I will look at the fabfile and see about adding the function back.

@minrk
Copy link
Member

minrk commented Apr 28, 2013

Just following up from IRC: this PR did not actually have any effect on the custom js mentioned above.

@epifanio
Copy link
Contributor

sorry for the noise ... was a path mismatch on my system, between different version of ipython
it works fine, js extension loaded correctly and the focus-click bug is no more here :)

@ellisonbg
Copy link
Member Author

The focus-click bug is definitely still in this branch. But there were
some CM bugs in <3 that were preventing us from fixing the focus-click bug.

On Sun, Apr 28, 2013 at 12:09 PM, epifanio notifications@github.com wrote:

sorry for the noise ... was a path mismatch on my system, between
different version of ipython
i confirm it works fine, js extension works fine and the focus-click bug
is no more here :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/3232#issuecomment-17140503
.

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@minrk
Copy link
Member

minrk commented Apr 28, 2013

jquery is not listed in the dependencies in bower.json

@ellisonbg
Copy link
Member Author

OK I have made all of the changes I know about: adding back the components fab function and added a README.md for notebook development.

minrk added a commit that referenced this pull request Apr 29, 2013
Update to CodeMirror 3 and start to ship our components

The main purpose of this branch is the update to CodeMirror 3 and ship it as a bower component. Because CodeMirror doesn't use proper semantic version tags, we have created a CodeMirror ipython fork. The sole purpose of this fork is for us to maintain properly named tags that bower can install from.

As a side effect of these changes, we are now shipping in place all bower components that we are using (bootstrap, less. codemirror, jquery). BUT, the version of jquery in components is not yet being used as we are still stuck with the old version attached to the nasty jquery.ui branch we ship. In a later PR, we will need to get rid of that jquery.ui branch (replace those capabilties with bootstrap) and then move to a stable version of jquery.ui that matches that of the jquery component depended upon by bootstrap.
@minrk minrk merged commit 60a66d4 into ipython:master Apr 29, 2013
@Carreau
Copy link
Member

Carreau commented May 1, 2013

Hum... lots of changes in 2 weeks, I would have preferred to be able to pitch in earlier.

I'll will re-do a suggestion I made earlier, which is to consider alternative to bower, which unlike bower allow to specify a tag or even a sha of a repository in the equivalent of component.json

Maintaining our own tagged codemirror branch totally goes against the point of have semantic version number and being able to specify any kind of version range. Also with this, if packager want to use another version, it is as difficult to install another version with bower than to close from the originial repository.

Unless I'm missing something..

@minrk
Copy link
Member

minrk commented May 1, 2013

We are not maintaining our own codemirror repo anymore, since we realized that bower cannot be used for everything

@ellisonbg
Copy link
Member Author

How are we managing CM then?

Sent from my iPhone

On May 1, 2013, at 8:52 AM, Min RK notifications@github.com wrote:

We are not maintaining our own codemirror repo anymore, since we realized that bower cannot be used for everything


Reply to this email directly or view it on GitHub.

@ellisonbg
Copy link
Member Author

I though we were still going to use bower to manage things within our single sub module

Sent from my iPhone

On May 1, 2013, at 8:52 AM, Min RK notifications@github.com wrote:

We are not maintaining our own codemirror repo anymore, since we realized that bower cannot be used for everything


Reply to this email directly or view it on GitHub.

@minrk
Copy link
Member

minrk commented May 1, 2013

Since we have proven that bower cannot install everything (CodeMirror, highlight.js, pagedown, prettify, etc.), that repo has two separate installation mechanisms - bower.json and nonbower.json, which is handled identically, but manually, for packages bower cannot install. Since we know that we cannot use bower for everything, I don't see a good reason to add our own repos for the ones that bower can't handle. That would mean we are maintaining our own repo as a submodule to track an installed clone of our own repo that is itself not even our own code, just for a stupid tag.

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Update to CodeMirror 3 and start to ship our components

The main purpose of this branch is the update to CodeMirror 3 and ship it as a bower component. Because CodeMirror doesn't use proper semantic version tags, we have created a CodeMirror ipython fork. The sole purpose of this fork is for us to maintain properly named tags that bower can install from.

As a side effect of these changes, we are now shipping in place all bower components that we are using (bootstrap, less. codemirror, jquery). BUT, the version of jquery in components is not yet being used as we are still stuck with the old version attached to the nasty jquery.ui branch we ship. In a later PR, we will need to get rid of that jquery.ui branch (replace those capabilties with bootstrap) and then move to a stable version of jquery.ui that matches that of the jquery component depended upon by bootstrap.
@Carreau Carreau mentioned this pull request Aug 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants