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

Include CodeMirror in SageNB and use it to edit data files #7501

Closed
williamstein opened this issue Nov 20, 2009 · 17 comments
Closed

Include CodeMirror in SageNB and use it to edit data files #7501

williamstein opened this issue Nov 20, 2009 · 17 comments

Comments

@williamstein
Copy link
Contributor

After an extensive evaluation, we all decided that CodeMirror is the best JavaScript code editor to include in Sage. It's faster and more robust than EditArea. Initially, we will include it only for editing Data --> file, then maybe later adapt it for input cells.

See this screenshot.

Apply both

to SageNB 0.7.4 (cf. #8051) OR get a trial spkg from #8194.

Component: notebook

Author: William Stein, Jason Grout, Mitesh Patel

Reviewer: Jason Grout, Tim Dumol

Merged: sagenb-0.8

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

@williamstein
Copy link
Contributor Author

Attachment: sagenb_7501-part1.patch.gz

Attachment: sagenb_7501-part2.patch.gz

@jasongrout
Copy link
Member

comment:1

Positive review to William's patch, as a proof-of-concept patch.

In the end, codemirror gets included on every page---that shouldn't be.

@jasongrout
Copy link
Member

Reviewer: Jason Grout

@jasongrout
Copy link
Member

Author: William Stein, Jason Grout

@jasongrout
Copy link
Member

comment:3

For the next step: these people apparently worked on an autocompletion function:

http://groups.google.com/group/codemirror/browse_frm/thread/37a078e69b26a213#

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 4, 2010

comment:4

I can rebase this and try to include it in SageNB 0.7.x (cf. #8051) or, perhaps, 0.8.

@qed777 qed777 mannequin added the s: needs work label Feb 5, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 5, 2010

comment:6

See the description for links to rebased patches. I've adjusted line numbers' style so they line up with the lines in the editor.

@qed777

This comment has been minimized.

@qed777 qed777 mannequin added s: needs review and removed s: needs work labels Feb 5, 2010
@qed777 qed777 mannequin changed the title notebook -- include codemirror in sage Include CodeMirror in SageNB and use it to edit data files Feb 5, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 7, 2010

comment:7

I should have said that the "rebased" patches are actually new. It seem easier to do this given the extent of changes to SageNB since the original patches were made. Also, the original set

  • Includes codemirror.js on every page.
  • Refers to parsesage.js and sage/css/pythoncolors.css, which seem to be missing. The new patches just use CodeMirror's Python parser and style.
  • Does not align the automatic line numbers with lines of text in the editor.

Part A just adds CodeMirror to the repository. Part B configures it for the data file page.

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 7, 2010

comment:8

Replying to @qed777:

  • Includes codemirror.js on every page.

Or more pages than necessary, at least.

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Feb 22, 2010

comment:9

Has any of this been tested on Solaris?

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 22, 2010

comment:10

Replying to @sagetrac-drkirkby:

Has any of this been tested on Solaris?

I haven't. But if you have a spare moment, please test the patches and let us know!

Note: CodeMirror runs entirely in the browser.

@qed777 qed777 mannequin added s: needs review and removed s: needs info labels Feb 22, 2010
@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Feb 22, 2010

comment:11

I would never have enough time to test individuals patches. But William is keen a Solaris port is completed very soon and if things don't get checked, then it hampers that port.

Dave

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Mar 19, 2010

Changed author from William Stein, Jason Grout to William Stein, Jason Grout, Mitesh Patel

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Mar 19, 2010

Changed reviewer from Jason Grout to Jason Grout, Tim Dumol

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Mar 19, 2010

comment:12

Looks ok to me, and it is extremely unlikely that this can have any negative effect on any platform, since, as Mitesh said, it's totally in the browser.

@TimDumol TimDumol mannequin removed the s: needs review label Mar 19, 2010
@TimDumol TimDumol mannequin added the s: positive review label Mar 19, 2010
@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Apr 24, 2010

Merged: sagenb-0.8

@TimDumol TimDumol mannequin removed the s: positive review label May 4, 2010
@TimDumol TimDumol mannequin closed this as completed May 4, 2010
@TimDumol TimDumol mannequin mentioned this issue May 3, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants