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

Replace ace with source-editor.jsm #11

Closed
simonster opened this issue May 21, 2012 · 11 comments
Closed

Replace ace with source-editor.jsm #11

simonster opened this issue May 21, 2012 · 11 comments

Comments

@simonster
Copy link
Member

Firefox 11 and later include a built-in source editor. We should use this instead of ace.

@rmzelle
Copy link
Collaborator

rmzelle commented Nov 4, 2014

That link says "This component has been removed from the platform in Firefox 28."

Looks like Firefox uses the CodeMirror code editor now: stylish-userstyles/stylish#141

@rmzelle
Copy link
Collaborator

rmzelle commented Jan 8, 2015

@rmzelle
Copy link
Collaborator

rmzelle commented Jan 10, 2015

I tried invoking the build-in CodeMirror, using stylish-userstyles/stylish@96f0d96 and the above link as guidance, but I couldn't get a CodeMirror editor window to render in Scaffold. @aurimasv, do you have some time to look at this?

Once that part works, it looks like the rest of the required code changes (mostly I/O to the editor windows) should be within my reach. (I would like to use CodeMirror for the Zotero Reference Test pane as well, FWIW)

@aurimasv
Copy link
Collaborator

Do you want to start a pull request (or just a branch on your fork of Scaffold) with the changes you have now and we can work up from there? (to be perfectly honest, I'm feeling a bit lazy and would rather debug your code than write it up from scratch)

@rmzelle
Copy link
Collaborator

rmzelle commented Jan 10, 2015

It's rather unfamiliar territory for me, so I didn't get very far.

It looks like Ace is initialized at https://github.com/zotero/scaffold/blob/master/chrome/content/scaffold/aceWrapper.js#L40 (and via https://github.com/zotero/scaffold/blob/master/chrome/content/scaffold/scaffold.js#L102 ?)

I also found iframes that call to the Ace library in scaffold.xul. E.g. https://github.com/zotero/scaffold/blob/master/chrome/content/scaffold/scaffold.xul#L130 unescapes to:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
   <head>
      <script type="text/javascript" src="chrome://scaffold/content/ace/ace.js"></script>
      <script type="text/javascript" src="chrome://scaffold/content/ace/mode-javascript.js"></script>
      <script type="text/javascript" src="chrome://scaffold/content/aceWrapper.js"></script>
   </head>
   <body id="body"></body>
</html>

@avram
Copy link
Collaborator

avram commented Jan 10, 2015

Sorry for the state of the Ace integration-- it was a bit of a hack and I knew nothing about XUL or Ace.

@rmzelle
Copy link
Collaborator

rmzelle commented Jan 10, 2015

Any chance you can take a look, @avram? :)

The benefit of switching to CodeMirror is arguably relatively minor for scaffold, but I was hoping to use it as a template for using CodeMirror in the Zotero Reference Test pane, which currently just has a text field, and where we probably wouldn't want to add a dependency for Ace.

@aurimasv
Copy link
Collaborator

There are actually good reasons to do it for Scaffold (e.g. incorporating a
debugger into scaffold, adding auto-completion, which may be possible
through ace, but I feel like CodeMirror would be easier, because Firefox
has done most of the work for us) and I've been looking into it a little
bit. It's just not at the top of the list for me right now, but I would
gladly support any such efforts.
On Jan 10, 2015 1:39 PM, "Rintze M. Zelle" notifications@github.com wrote:

Any chance you can take a look, @avram https://github.com/avram? :)

The benefit of switching to CodeMirror is arguably relatively minor for
scaffold, but I was hoping to use it as a template for using CodeMirror in
the Zotero Reference Test pane, which currently just has a text field, and
where we probably wouldn't want to add a dependency for Ace.


Reply to this email directly or view it on GitHub
#11 (comment).

@dstillman
Copy link
Member

Any further thoughts on this? One way or another we'll want to switch to unminified files before submitting the XPI for signing to avoid hearing from Mozilla later, and right now all the ACE files are minified.

@dstillman dstillman mentioned this issue Dec 10, 2015
dstillman added a commit that referenced this issue Dec 12, 2015
Removes all the old theme files, which I don't think were used (?), and
switches to Monokai as the default theme

Addresses #11 and #24
@rmzelle
Copy link
Collaborator

rmzelle commented Dec 21, 2015

Any further thoughts on this?

It might still be worthwhile to replace ACE with the CodeMirror editor included in Firefox (and use it for csledit.xul as well). I still can't figure out how to load the Firefox editor, though. If I use the instructions at https://developer.mozilla.org/en-US/docs/Tools/Editor
let Editor = require("devtools/sourceeditor/editor");
in Zotero_CSL_Editor.init in csledit.js, I get the error
JavaScript error: chrome://zotero/content/tools/csledit.js, line 31: ReferenceError: require is not defined.

I figured I might have to do something like
Components.utils.import("resource:///modules/devtools/sourceeditor/editor.js");
, but that gave the error
JavaScript error: resource:///modules/devtools/sourceeditor/editor.js, line 9: ReferenceError: require is not defined

PS. I just found https://github.com/nt1m/devtools-prototyper/search?utf8=%E2%9C%93&q=sourceeditor, which seems to use the CodeMirror editor.

@zuphilip
Copy link
Collaborator

zuphilip commented May 5, 2018

We activated JSLinter, Search and Replace feature in ACE and are working on the Autocompletion. It does not seem to be necessary anymore to switch away from ACE.

@zuphilip zuphilip closed this as completed May 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants