senny / cabbage

get the maximum out of emacs
http://senny.github.com/cabbage/
156 stars 21 forks source link

Ditching skeleton.el in favor of autopair #92

Closed fred-o closed 12 years ago

fred-o commented 12 years ago

While struggling to bend skeleton.el to my will I discovered autopair, which seems like a better way of accomplishing the same thing. It is designed to mimic textmate as much as possible and has nifty features such as skipping through the closing end of a pair when typing it, and deleting a newly created pair with just one backspace (very useful if you often mistype parens and braces, like I do).

Check it out at http://code.google.com/p/autopair/

senny commented 12 years ago

how does it handle different paring characters for different modes like we had with skeleton.el?

fred-o commented 12 years ago

It claims to deduce a reasonable default of pairs for the different modes. I haven't used it extensively, so I can't tell you how well it works for all of them. The java, elisp and xml defaults seem okay, though.

There are mechanisms for customizing autopair for different modes, for example handling triple-quotes in python.

senny commented 12 years ago

I need to verify this before merging, I think everyone gets into habits with paren-managing keystrokes. If it does not conflict with my current handling I'm fine to merge this.

fred-o commented 12 years ago

After playing with it a bit longer, I noticed that it doesn't pair the '|' character for ruby-mode. There is a workaround, though. I'll give it a shot later today.

fred-o commented 12 years ago

Oh, just noticed that autopair seems to break auto-complete-mode, so that you can't use RET from the menu to choose a completion. I'll see if there's a workaround, otherwise that's kind of a dealbreaker for me.

senny commented 12 years ago

@fred-o what did you not like about skeleton.el in the first place? I think it does a good job in inserting pairs and I never felt it lacked something...

fred-o commented 12 years ago

Well, I experienced some strange behaviour where it didn't work as expected. Also, two features that I miss are the ability to intelligently skip over the closing part of a pair (typing ')' when at the end of a parenthesis should skip over the existing ')' instead of inserting a new one), and removing the whole newly created pair with a single backspace when you mistype (this is a minor issue, but it's really, really convenient).

fred-o commented 12 years ago

Ok, I think I found a workaround for the autopair issue. Also added pairing of pipes and triple quotes to ruby and python modes.

jone commented 12 years ago

There are some issues with rebinding RET - it's not working in terminal any more - I think autopair should be disabled in terminal bundle.. Also I'd like to have backticks to autopair in python mode :) but I can add that myself.. Maybe it would be nice to have a function for adding additional characters per mode?

In general the behavior feels good. :+1:

fred-o commented 12 years ago

Done and done! :)

jone commented 12 years ago

cool :) @senny, did you test it? +1 on merging ;)

senny commented 12 years ago

@jone not yet. I think I'll get to it today.

senny commented 12 years ago

most things work for me. That closing a paren in front of another closing paren skips over it is something i must get used to though. The pairing behavior seems rather laagy to me. I always see the cursor blinking around which isn't that big of a deal but it annoys me.

I also noticed that I miss some paring for "haml": single quotes ' and pipes are not paired.

fred-o commented 12 years ago

I've added singlequote and pipe pairing to haml-mode.

If you set autopair-blink to nil, it will disable blinking. Maybe this should be the default?

senny commented 12 years ago

@fred-o I'll give it another shot later today.

senny commented 12 years ago

i just wanted to perform a replace-string and noticed that it inserts double "" even in the replace-string call in the minibuffer. I think this is a step backward from sekelton.el, which operated mostly calculated and was easily controlled.

fred-o commented 12 years ago

Personally I don't have a problem with autopair inserting a double "" in the minibuffer, but you can turn it off with

(add-hook 'minibuffer-setup-hook 
          (lambda () (setq autopair-dont-activate t)))
senny commented 12 years ago

@fred-o I really need to use this branch for a couple of days to ensure things can be controlled to behave as I want them to ;) can you rebase the branch against the current master branch so that we got an up-to-date version.

//cc @jone

fred-o commented 12 years ago

My git-fu is weak, but I think I managed to rebase the branch :)

senny commented 12 years ago

@fred-o I've used this branch for some time now and the pairing behaves very nicely. There is a huge problem though which prevents me from merging the pull-request. When using the autopair branch I occasionally get buggy behavior with selections. In the middle of the session I can't replace selections anymore. I always have to delete them before inserting text. I can fix the problem by calling delete-selection-mode twice but it resurfaces after some editing time. I can't reconstruct the behavior but I get it in nearly every session. I talked to @jone and he noticed the same bug.

jone commented 12 years ago

I think there is a problem with the advice of delete-selection-pre-hook in autopair.el:

(defadvice delete-selection-pre-hook (around autopair-override activate)
  "Don't actually do anything if autopair is about to autowrap. "
  (unless (autopair-should-autowrap) ad-do-it))

(defun autopair-should-autowrap ()
  (let ((name (symbol-name this-command)))
    (and autopair-mode
         (not (eq this-command 'autopair-backspace))
         (string-match "^autopair" (symbol-name this-command))
         (autopair-calculate-wrap-action))))
jone commented 12 years ago

When the bug occurs, removing and re-adding the delete-selection-pre-hook eliminates the bug:

(remove-hook 'pre-command-hook 'delete-selection-pre-hook)
(add-hook 'pre-command-hook 'delete-selection-pre-hook)

Maybe the hook is suddenly not registered anymore...

phgross commented 12 years ago

I have exactly the same problem, like jone describe above wit the delete-selection-pre-hook. Removing and re-adding the pre hook elminates the bug.

fred-o commented 12 years ago

Hm... I can't say I've noticed this bug (or if I have, I can't remember it). The last section of autopair.el mentions monkey-patching to get delete-selection-mode and cua-mode working properly, which does not sound that reassuring :(

There seems to be a few issues reported regarding delete-selection-mode and autopair, but I couldn't find one that matched this problem.

senny commented 12 years ago

@fred-o I discussed this pull-request with @jone today. We both feel that there are too many glitches with using autopair and that we don't feel any particular need to replace skeleton.el. With e-max we want to give Emacs users a good entry point and clear opinions about what we think is worth using. This does not mean that you should have the same opinion though and we want to get e-max as compatible as possible. Regarding this issue. What needs to be done, that you can activate autopair simply in your ~/.emacs.d/local.el configuration?

fred-o commented 12 years ago

Off the top of my head, I don't think any changes are needed. I think all I have to do is deactivating skeleton.el (which can be done by setting e-max-insert-pairs to nil), then loading autopair.el.

senny commented 12 years ago

@fred-o please let me know if you can get it working on the latest master branch with simply deactivating e-max-insert-pairs. It would also be nice to post the integrating code from your local.el to this pull-request (inline code) for other people to use it. I'll close this one when you got it working.

fred-o commented 12 years ago

Assuming autopair.el is downloaded from http://autopair.googlecode.com/svn/trunk/autopair.el and stored in ~/.emacs.d/vendor/autopair, putting this in local.el does the trick:

(add-to-list 'load-path "~/.emacs.d/vendor/autopair")
(require 'autopair)
(autopair-global-mode)
(setq e-max-insert-pairs nil)
senny commented 12 years ago

@fred-o amazing! this looks very much like the normal autopair setup (except the e-max-insert-pairs) of course. I think anyone with a little bit of emacs and elisp knowledge is able to perform the installation.

I think we should use e-max as an opinionated basic setup, which gets you up to speed in no time like the emacs-starter-kit. On the other hand, we want the users to continue using e-max even when they've made their first steps. We don't want that they start over with a blank configuration and copy the parts they liked. We should design e-max to integrate well with third party libraries so that it is a good foundation to build your own configuration upon.

closing this. E-max will ship with skeleton.el and can be easily used with autopair as described above.