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

Aceify #1264

Merged
merged 3 commits into from Jan 18, 2012
Merged

Aceify #1264

merged 3 commits into from Jan 18, 2012

Conversation

ellisonbg
Copy link
Member

Added the ability to edit a single code cell nearly full screen in an Ace editor. Ace is a more full featured code editor than CodeMirror, but you can only have one on a page, so we can't use it for cells. This should improve the use experience when working with large code cells. In the long run we can add different configuration
options to this editor.

@ellisonbg
Copy link
Member Author

@fperez: I think I have fixed the issue you were seeing. Please give it a try and let me know.

@fperez
Copy link
Member

fperez commented Jan 18, 2012

This is great! I like it a lot, and I can see moving forward this providing also support for editing .py files in the notebook directory (and even others). This would be great for teaching scenarios where we've always struggled with the 'what editor do I use' question, as well as for remote usage where everything could be done via ipython regarding managing of normal scripts, shell scripts, etc. So a big +1 to the whole thing.

A few things I've noticed in quick testing, I don't know how easy they will be to fix:

  • the font changes compared to the one in the normal cells, and it shouldn't. We should keep the font experience consistent throughout.
  • tab completion doesn't work at all in the Ace cells.
  • search and replace only works one instance at a time, there's no 'replace all' option in the dialog. Also, the search and replace dialog shows 'find' first and then 'replace' in a new dialog, I don't know if we have control over that so we could show a more traditional dialog with both the 'find' and 'replace' options in a single screen.
  • no menu/buttons/anything suggesting to users what the keybindings are. I found the ones for find and replace by blind trying.

The first two issues I think we should really fix before merging. The others are icing on the cake, and I'm fine if they have to wait for improvements in Ace itself (though obviously it would be great to get them done if possible).

Great job in all!

@ellisonbg
Copy link
Member Author

Fonts: good catch, should be a simple fix.
Tab completion: this will require a major refactor of the completion code in the CodeCell class. Unfortunately the recent additions (tooltips and filtering) made an even worse mess out of already messy code. This code is incomprehensible, exceedingly complex and all around welded to CodeMirror. All that to say, there is no way I have the time to implement tab completion for the Ace editor right now and we should definitely not try to hack it in and make even more of a mess. I would rather merge this branch now as is and open an issue for the completion refactoring.
Search and replace: ha, what search and replace? More seriously, I hadn't poked around to find keyboard shortcuts yet. I would have to look into the implementation of search and replace in Ace. Unfortunately, their docs are quite thin and the code is huge. Definitely one for later.
Keybindings: I will look into this one - at least some hint of what they are would be good.

@fperez
Copy link
Member

fperez commented Jan 18, 2012

Sounds good. It's a bummer the tab completion is complex to fix, but such is life. I agree, much better to leave as a separate issue to refactor and clean up that code to a point in which it can be easily reused from either the CM or Ace cells.

Re. keybindings, the only ones I've found so far are Tab/Shift Tab for indent/dedent (you told me these), C-f for find, C-r for search & replace. There may be others :)

I also saw on their site that they have emacs/vi modes, having this as a menu option would be great. Having a vi mode will really appeal to the vi true believers, b/c for them editing in 'normal' editors is so different from their muscle memory.

Though if exposing that configurability is best done once we have tackled the more generic config question in the browser, that's ok too.

* The editor won't get focus on some browsers (Chrome/FF on Linux and Safari
on Mac). Latest dev master seems better but the issue still shows up
sometimes. Adding a manual window.resizeBy fixes this for now.
* Font size and family matches that of our CodeMirror editors.
* Help link to Ace keyboard shortcuts added to help.
@ellisonbg
Copy link
Member Author

I have opened a ticket for the refactoring of the tab completion/tooltip stuff: #1287

I have matched the fonts to the CodeMirror editors.

I have added a link in our help menu to the Ace keyboard shortcuts (there are tons of them!).

@ellisonbg ellisonbg merged commit dd08faf into ipython:master Jan 18, 2012
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

2 participants