Alphanumeric cell IDs, resize on paste (#2902), ESC ends introspection (#5644), JSLint for notebook_lib.js

[qed777]

This ticket

• Makes it possible to assign alphanumeric cell IDs.
• Should subsume
  • notebook -- resize of cell should also fire on paste into the cell #2902 - Resize an input cell after a paste event.
  • make it so escape make the introspection window in the notebook disappear #5644 - Make ESC close introspection windows.
• Runs notebook_lib.js "through" JSLint.

Patch:

• http://boxen.math.washington.edu/home/mpatel/trac/7666/trac_7666-alphanumeric_cell_ids_B5.patch

Suggested queue, applied to SageNB 0.4.9 + #7269:

trac_7843-cell_listdir.patch
trac_7844-notebook_address.patch
trac_7847-empty_trash_ie_ff.patch
trac_7846-no_CODE_PY_symlinks_v2.patch
trac_7650-sagenb_doctesting_v4.patch    # Possible trivial failed "hunk" here.
trac_7786-template-jinja-idiomatic.12.patch
trac_7871-interact_bgcolor.patch
trac_7858-key_binding_vars_v2.patch
trac_7863-declare_vars_aux_js_v2.patch
trac_7874-typeset_interact_labels.patch
trac_7666-alphanumeric_cell_ids_B5.patch

Or install this spkg:

• http://boxen.math.washington.edu/home/mpatel/trac/7666/sagenb-0.5-7666b5.spkg

CC: @williamstein @TimDumol @boothby @mwhansen

Component: notebook

Author: Mitesh Patel

Reviewer: Tim Dumol

Merged: sagenb-0.6

Issue created by migration from https://trac.sagemath.org/ticket/7666

[qed777]

Author: Mitesh Patel

[qed777]

comment:2

Please the commit string for a brief summary. Other notes:

• Depends on notebook: move preparsing to the worksheet process and out of the server (was: weird pointless line) #7483, provide a mode so that undeclared variables magically spring into existence and object oriented notation is not necessary #7482, rewrite load and attach #7514, Fix sagenb doctesting #7650, SageNB -- Change table layouts to CSS layouts #7269. Negligible dependence on notebook: issue with indentation of first line being wrong #7648.

• It seems that auto-indentation was already broken in IE8. V9 does not fix the problem.

• Possible CSS problem: The rendered completions menu is just two columns (not characters) wide in IE8 and O10.

• On introspection: I think I've reproduced most of the standard behavior. New: Pressing TAB for explicitly requested docstrings or source code toggles between both.

• We might use jQuery's data store to keep cells and their variables together.

• IE8 CPU problem: jsMath seems to be involved, somehow. Try factor(factorial(100)), say, and scrolling up and down.

[qed777]

comment:3

A pre-existing bug:

• "Evaluate All" cells in a worksheet.
• Evaluate the last cell.
• A new cell is not automatically inserted in the notebook.
• Reload to see the new cell.

? It seems the client and server cell lists are out of sync. This might explain the "spontaneously appended cell" phenomenon.

[qed777]

comment:4

In v9, "Evaluate All" can delay the updates of freshly rendered interacts. I can restore their priority (e.g., prepend their IDs to the active_cell_list) here or at #6855.

[qed777]

comment:5

Replying to @qed777:

A pre-existing bug:
[...]
? It seems the client and server cell lists are out of sync. This might explain the "spontaneously appended cell" phenomenon.

The "B"-series of patches at #6855 should fix this problem.

[qed777]

comment:6

V10 at

• http://sage.math.washington.edu/home/mpatel/trac/7666/

is rebased vs. Sage 4.3, SageNB 0.4.8, #7483, #7482, #7514, #7650, #7269.

[qed777]

comment:7

The latest, V13, is rebased vs. #7811.

[qed777]

comment:8

I'll stop pre-emptively rebasing this patch, for now. Just let me know, if/when it's time.

[williamstein]

comment:9

mpatel -- fyi, Tom Boothby is going to help get this work into Sage very soon...

[qed777]

comment:10

That's great! I should still rebase the patch and include a few corrections from #6855's "B" series. Remarks:

• I've made many changes, it seems. Absolutely no offense is intended.
• It's likely the introspection rewrite is just a first step.
• The JS functions are not consistently documented. I hope to fix this in a future ticket.
• It's likely that many Selenium tests on non-FF browsers will now fail. (Selenium is problematic, not everyone is testing them, etc.) I won't fix them here.
• I think we can write/use a simple JS doctesting framework for non-{DOM, network} functions, but this should be a separate ticket.

[qed777]

comment:11

Just a quick note: I'm working on breaking this up into more manageable pieces.

[qed777]

comment:12

Replying to @qed777:

Just a quick note: I'm working on breaking this up into more manageable pieces.

With Tim's permission, I've added the Se test framework changes to #7786.

[qed777]

comment:13

Please see #7858 about setting the Content-Type header for dynamic JavaScript files.

[qed777]

comment:14

#7863 applies JSLint to the notebook's auxiliary JS files.

[qed777]

comment:15

If I save $\alpha$ in a text cell, then edit the cell again, I see Ë in the editor. Moreover, the HTML source now contains <span> tags, etc.

[qed777]

comment:16

Oops! Wrong ticket.

[qed777]

comment:17

• Try giving names/labels to cells in the "Edit" window, e.g.,

  `{{{id=hello|
  factor(factorial(37))
  ///
  }}}`
  

* [JSLint](http://www.jslint.com/) is not a compiler, but it can find bugs.  Of course, it helps to start with a reasonably lint-free file!  To do: Look into [Closure Tools](http://code.google.com/closure/).

[qed777]

comment:19

Reminder: Set proxify to false prior to merge (if we get that far).

[rbeezer]

comment:20

mpatel asked if I would test this package. Some observations follow, Firefox 3.5.6 on Kubuntu 9.10.

1. ESC on introspection seems to be working (a great addition!). First time I hit ESC it didn't seem to have effect, then next tab-completion was weird, then all subsequent uses of ESC worked fine. Opened a new worksheet, and several uses of ESC worked fine. So maybe just once per server session?

2. Formatted labels are working in interacts, but now I don't see any output. Three different interacts, no output at all. Controls are functional though, so it is not hung.

3. Was testing resizing cells on paste:

(a) paste into a new cell, hit edit key and the pasted text is gone.

(b) this one took me a while

• make a new cell, type a space, then type a sentence almost to the far end of the cell.

• resize browser window by grabbing right edge and dragging left (slowly)

• resize until just the last word of sentence wraps to a new line.

• cell doesn't resize and second line is hidden

• hit edit, cancel changes, see cell is now sized right

• click into cell, it sizes wrong again

Without a space at the start of the line, sometimes there's no problem. Notice if you keep sliding the window edge, and push two words onto the next line, the sizing comes back to being right.

[qed777]

comment:21

Replying to @rbeezer:

mpatel asked if I would test this package. Some observations follow, Firefox 3.5.6 on Kubuntu 9.10.

Thanks for your detailed feedback!

1. ESC on introspection seems to be working (a great addition!). First time I hit
2. Formatted labels are working in interacts, but now I don't see any output.

I think the JavaScript compressor may be so aggressive that it's changing the semantics of the code. Could you try this (but don't bother if it's too much trouble):

• Insert debug_mode = False just before _cache_javascript = None in $SAGE_ROOT/local/lib/python2.6/site-packages/sagenb-0.5-py2.6.egg/sagenb/notebook/js.py.
• Restart the server, clear the browser's cache, and test ESC and interacts again.

3. Was testing resizing cells on paste:
   (a) paste into a new cell, hit edit key and the pasted text is gone.

This should be easy to fix by also sending the input to the server just after the paste event.

(b) this one took me a while

I've noticed this, too. The server code and JS library use different methods to (re)size a cell. The server does the sizing just before it sends the worksheet to the browser (e.g., on first display, after exiting "Edit" mode, etc.), but the browser resizes in between. I don't know how easy it is to reconcile the methods, but I'll definitely take a closer look.

[qed777]

comment:22

Oops. That should be debug_mode = True.

[qed777]

comment:23

V4 should make it so a cell is resized and saved about 0.25 seconds after a paste event -- using no delay at all is too fast, apparently.

I think unifying the client and server resize methods should be a separate ticket. Neither of the current methods is optimal. The server assumes 80-column-wide input cells. The browser suffers from the problem described above.

I'm not sure what to do about the compressor. See #7787 and sage-notebook for potential options. For now, how about adding .min.js file(s), generated offline with the YUI Compressor?

[qed777]

comment:25

Found the problem: The JavaScriptCompressor converts, in effect,

function foo() {
    return 'hello';
}

to

function foo(){return 
'hello'
;}

But

In JavaScript, a linefeed can be whitespace or it can act as a semicolon. This replaces one ambiguity with another.

In particular, if I execute foo();, the former returns 'hello' and latter returns undefined. I've modified the compressor in V5 so that it does not insert extra '\n's. Of course, we should replace this with an free / open source, stable, Pythonic minifier (see above), when we can. By the way, the YUI Compressor yields

function foo(){return"hello"}

[qed777]

comment:27

I've updated the spkg to V5, too.

[TimDumol]

comment:28

Excellent work. Cleans up the library quite well.

I've rebased the patch on a new patch queue, but since the reject is just some empty lines, I'll mark this with a positive review.

--- notebook_lib.js
+++ notebook_lib.js
@@ -4376,10 +4676,10 @@ function decode64(input) {
     return output;
 }
 
+
 ///////////////////////////////////////////////////////////////////
 // Trash
 ///////////////////////////////////////////////////////////////////
-
 function empty_trash() {
     /*
     Empties the trash folder, after asking for confirmation.

The new patch queue is:

trac_7650-sagenb_doctesting_v6.patch
trac_7650-reviewer.patch
trac_7648-missing_indent.patch
trac_7663-rstrip_prompt.patch
trac_7847-empty-trash-no-referer.patch
trac_7786-template-jinja-idiomatic.patch
trac_7863-declare_vars_aux_js_v2.patch
trac_7874-typeset_interact_labels.patch
trac_7858-key_binding_vars_v2.patch
trac_7666-alphanumeric_cell_ids_B5.patch

[TimDumol]

Rebase on a new patch set.

[TimDumol]

Attachment: trac_7666-alphanumeric_cell_ids_B5.patch.gz

Attachment: trac_7666-reviewer.patch.gz

A few style tweaks.

[TimDumol]

comment:29

I've attached a few additional style tweaks to be applied on top of the patch. Feel free to review it or ignore it.

[qed777]

Combined patch. See comment. Apply only this patch.

[qed777]

comment:30

Attachment: trac_7666-alphanumeric_cell_ids_B6.patch.gz

I've attached a combined patch that incorporates most of the reviewer patch. Thanks for further simplifying handle_introspection! If it's OK, I'd like to adhere to Crockford's one-var-statement-per-function rule (see Scope), to minimize false positives from JSLint on its "The Good Parts" setting.

[qed777]

Reviewer: Tim Dumol

[TimDumol]

comment:31

I think that the rule is made for functions with less than say, 5, variables. Putting all of those declarations in one line is a tad hard to read.

[qed777]

comment:32

B7 does this

function bar() {
    var foo1, foo2, foo3, foo4, foo5, foo6,
        foo7, foo8, foo9, foo10;
    // ...
}

[qed777]

Attachment: trac_7666-alphanumeric_cell_ids_B7.patch.gz

Combined patch. Apply only this patch.

[TimDumol]

Merged: sagenb-0.6