Updatecm #1303

ellisonbg · 2012-01-20T21:00:33Z

This updates CodeMirror and refactors a good bit of the notebook code.

Updated CodeMirror to the latest stable release.
Fix numerous bugs related to the CM update.
Refactored the Cell API and the notebook's cell handling methods.
Generalized split/merge to work with all cell types.
Generalized "Edit in Ace" to work with all cell types.
Loading optimizations: pager starts out hidden, faster loads.
Shading added to Markdown and HTML cells when they are being edited.

This branch will require solid usability testing on Safari, FF and Chrome before merging.

Carreau · 2012-01-20T21:56:48Z

Chrome 16, Mac 2 blinking cursor:

have 2 cell.
click in the first
edit in ace
close
click the second
first have still the blinking cursor

100% broken in safari, (full white page, some element on display none)

  ace.js:1SyntaxError: Cannot declare a parameter named 'e' in strict mode 
  mode-python.js:1ReferenceError: Can't find variable: define
  mode-markdown.js:1ReferenceError: Can't find variable: define
  mode-html.js:1ReferenceError: Can't find variable: define
  theme-textmate.js:1ReferenceError: Can't find variable: define
  fulleditwidget.js:32ReferenceError: Can't find variable: ace

fperez · 2012-01-20T22:02:13Z

Sorry I can't test in detail now, but I tried to use it for work and noticed a problem, shown here:

I got this by going to ace, typing a two-line cell, and back to normal with C-m e. It created one spurious blank cell (the no. 2 cell shouldn't be there) and also truncated my cell at the bottom, which should have two lines. This was on chrome, with multiple page reloads to ensure I had the right JS.

Carreau · 2012-01-20T22:08:23Z

Note, ace.js is a minified source, which would gave the sames problems has the minified source of jquery for shipping with debian.

fawce · 2012-01-20T22:37:25Z

testing scope

I only had time to try a subset of the new functionality - merging code cells, editing code cells in ACE. I didn't have time to try the markdown and html cells.

usability nits

my first guess for "edit in ace" was under the edit menu...
when i went to try merging for the first time, i first tried to multi-select cells. took me a few tries to find the right menu item.
I looked at ace as well, and I found it a bit buggy in their kitchen sink demo. What is the benefit of using ACE vs. just styling up Codemirror? You can get line numbers, error/break markers, and anything else in CodeMirror as well.
when I switched to edit a cell in ACE, I thought something had gone wrong, like i had lost my other cells. once you are editing in ACE, it is a little scary to click a close button without saving first. maybe you could just plop the ACE editor right into the same div as the original codemirror cell?
was there always a BUSY signal in the top right? I love it.
refreshing page after working on a new notebook for a while actually creates a new notebook. /new should probably redirect to the new notebook so that /new and opening an existing notebook have the same url context. also good for getting the permalink to the actual notebook

functional bugs

menu header selection bug:

click on menu header (e.g. file), header is selected, menu displayed
click on same menu hader again, menu disappears but header is still selected
clear by selecting an item in the same menu, or picking another menu (e.g. file --> save)
quick screen capture: http://screencast.com/t/A9v1OptTki

blank file name accepted for notebook:

click notebook title, delete all text
save notebook with blank name
go to notebook list, your original notebook is gone

first time load of a notebook after upgrading from 0.12, requires a refresh for the javascript

design ideas

maybe when a cell is selected, you could add a little tab to the div with extra controls to move it up/down, ACE edit it, merge it
multi-selecting with ctrl-mouseclick, or shift+arrow keys like you do in excel would be cool. maybe this would work well with the tool tab idea above. or maybe there's a hotkey to merge the cells that are selected

ellisonbg · 2012-01-22T21:47:53Z

@fperez: yes this is a bug and I think I know how to fix it. Related to all of the CodeMirror issues I am having.

@Carreau: yes, we will need to add the unminified source.

@fawce:

We should better understand why Ace is better or not. CodeMirror has improved a lot so I am not sure it makes sense to have a Ace mode. Also, we could make the editor a dialog which would show the original notebook cells in the background.
Maybe name the "Close" button "Return to Notebook" or something?
Only one Ace editor is allowed on the page, which makes it difficult to inject into the cells.
We should have multiple select.
The bugs in the Menu are on the jQueryUI side of things. We are using beta quality code and I expect it to improve over time.

fperez · 2012-01-22T22:33:18Z

@ellisonbg, glad you can reproduce it. Let me know and I can keep on testing...

fawce · 2012-01-23T01:57:10Z

@ellisonbg - Return to Notebook sounds good to me.

* Added set_text/get_text in favor of get_code/set_code and get_source/set_source. * Added methods to the base class (get_text, set_text, refresh, edit, render, toJSON, fromJSON).

* Added placeholder to the base Cell class. * Removed the \u0000 char at the beginning of the TextCell placeholder that was there for a CodeMirror bug workaround. * Other refactoring of Cell related Notebook methods.

* Cell insertion code completely rewritten. We now have two methods rather than 9: insert_cell_above, insert_cell_below. * Renaming methods to be more consistent. * Creating new methods to provide a uniform API. * Minor bug fixes.

* Merge/split works for all cell types. * Notebook.select won't select an already selected cell. * Bugs in markdown cell editing fixed. * border-box-sizing used for Markdown cells to get correct css boxes. * Shading/border added to Markdown cell editor.

ellisonbg · 2012-01-23T19:33:56Z

OK I think I have fixed the main bugs, please test this branch on all platforms/all browsers.

ellisonbg · 2012-01-23T20:04:06Z

The bug on Safari is with Ace. It looks like there is a fix for this, but we need to decide if we want to keep using Ace.

ellisonbg · 2012-01-24T03:21:28Z

I am going to merge this. I am pretty sure I have resolved all of the non-Ace related bugs, which are also in master. I have 3 other branches whose progress depends on this branch and things are getting impossible to manage and move forward. We will need to figure out the Ace stuff in a separate branch.

* Block-level indent/dedent works. * Tab in docstrings works.

This updates CodeMirror and refactors a good bit of the notebook code related to it. * Updated CodeMirror to the latest stable release. * Fix numerous bugs related to the CM update. * Refactored the Cell API and the notebook's cell handling methods. * Generalized split/merge to work with all cell types. * Generalized "Edit in Ace" to work with all cell types. * Loading optimizations: pager starts out hidden, faster loads. * Shading added to Markdown and HTML cells when they are being edited. * This branch will require solid usability testing on Safari, FF and Chrome before merging. * Fixed a number of CM related bugs.

Carreau · 2012-01-24T10:02:36Z

Question,
I've just seen that you now have each notebook saved date, but for loaded notebook, it is the date/time at which it was loaded. Is it planed to changed to the "Real" date of saving ? Or at least the start with a "Loded on January..." then changed to "last saved..." ?

Otherwise splitting/merging cell is really cool, but I tend to search for it in the 'Cell' menu.
still completly broken with safari...

Carreau · 2012-01-24T10:27:12Z

Replacing all use strict in IPython/frontend/html/notebook/static/ace/ace.js by empty string seem to work fir safari.
One of them might be 'leaking' into global namespace

ellisonbg · 2012-01-24T21:22:12Z

Yes, the fix is easy, but I think we are going to move away from Ace...

On Tue, Jan 24, 2012 at 2:27 AM, Bussonnier Matthias
reply@reply.github.com
wrote:

Replacing all use strict in IPython/frontend/html/notebook/static/ace/ace.js by empty string seem to work fir safari.
One of them might be 'leaking' into global namespace

Reply to this email directly or view it on GitHub:
#1303 (comment)

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

This updates CodeMirror and refactors a good bit of the notebook code related to it. * Updated CodeMirror to the latest stable release. * Fix numerous bugs related to the CM update. * Refactored the Cell API and the notebook's cell handling methods. * Generalized split/merge to work with all cell types. * Generalized "Edit in Ace" to work with all cell types. * Loading optimizations: pager starts out hidden, faster loads. * Shading added to Markdown and HTML cells when they are being edited. * This branch will require solid usability testing on Safari, FF and Chrome before merging. * Fixed a number of CM related bugs.

ellisonbg added 10 commits January 23, 2012 11:33

Updating to CodeMirror 2.2, latest stable release.

7d2dff1

Fixing bugs that have shown up since updating CM to 2.2.

911dba0

Work on the base Cell API.

f790de5

* Added set_text/get_text in favor of get_code/set_code and get_source/set_source. * Added methods to the base class (get_text, set_text, refresh, edit, render, toJSON, fromJSON).

Updating cell logic.

5281dc6

* Added placeholder to the base Cell class. * Removed the \u0000 char at the beginning of the TextCell placeholder that was there for a CodeMirror bug workaround. * Other refactoring of Cell related Notebook methods.

Refactoring of the notebooks cell management.

fd82b35

* Cell insertion code completely rewritten. We now have two methods rather than 9: insert_cell_above, insert_cell_below. * Renaming methods to be more consistent. * Creating new methods to provide a uniform API. * Minor bug fixes.

Ace editor now works with Markdown and HTML cells with proper modes.

cbd3102

Start the pager out collapsed.

9ad2978

Optimizing notebook loading.

197288e

Fixing bugs in CodeMirror refreshing.

5caff73

Adding refresh/focus/refresh back to edit to fix bug in FF.

9617295

Disallow empty notebook names.

c4af222

ellisonbg mentioned this pull request Jan 23, 2012

Insertcell #1314

Closed

Fixing broken notebook saves.

ea54096

Fixing auto-indent issues in CodeMirror config.

4e3ae12

* Block-level indent/dedent works. * Tab in docstrings works.

ellisonbg merged commit c585349 into ipython:master Jan 24, 2012

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updatecm #1303

Updatecm #1303

ellisonbg commented Jan 20, 2012

Carreau commented Jan 20, 2012

fperez commented Jan 20, 2012

Carreau commented Jan 20, 2012

fawce commented Jan 20, 2012

ellisonbg commented Jan 22, 2012

fperez commented Jan 22, 2012

fawce commented Jan 23, 2012

ellisonbg commented Jan 23, 2012

ellisonbg commented Jan 23, 2012

ellisonbg commented Jan 24, 2012

Carreau commented Jan 24, 2012

Carreau commented Jan 24, 2012

ellisonbg commented Jan 24, 2012

Updatecm #1303

Updatecm #1303

Conversation

ellisonbg commented Jan 20, 2012

Carreau commented Jan 20, 2012

fperez commented Jan 20, 2012

Carreau commented Jan 20, 2012

fawce commented Jan 20, 2012

testing scope

usability nits

functional bugs

design ideas

ellisonbg commented Jan 22, 2012

fperez commented Jan 22, 2012

fawce commented Jan 23, 2012

ellisonbg commented Jan 23, 2012

ellisonbg commented Jan 23, 2012

ellisonbg commented Jan 24, 2012

Carreau commented Jan 24, 2012

Carreau commented Jan 24, 2012

ellisonbg commented Jan 24, 2012