joaotavora / autopair

Automagically pair braces and quotes in emacs like TextMate
208 stars 35 forks source link

Autopair prefs should be declared with defcustom #24

Closed DarwinAwardWinner closed 11 years ago

DarwinAwardWinner commented 11 years ago

It would be nice to access autopair's preferences via M-x customize. I would be willing to do most of the work in converting most of the variables meant for user customization to be declared with defcustom. Is this something you'd be interested in merging if I did the work?

DarwinAwardWinner commented 11 years ago

I've addressed the low-hanging fruit in a97d6b7. Tricker ones are the buffer-local variables autopair-dont-activate, autopair-extra-pairs, and autopair-dont-pair. If I understand correctly, these are intended to be set in major mode hooks to appropriate values for a given major mode. Thus obviously a single global customize value is not appropriate for these. Instead, these variables could be change to be alists where keys are major mode names and values are the current values, and then after-change-major-mode-hook could be used to set the buffer-local versions accordingly.

joaotavora commented 11 years ago

I've merged a97d6b7 using a pull request.

You are also right in observing that those remaining variables cannot be defcustoms are are intented to be set in the major-mode's hook. However, due to a recent change (well, not so recent) in the way emacs handles major-mode hooks, the autopair-dont-activate variable won't really work since when the major mode hook sets it, autopair has already launched. In other words any variable affecting autopair's initialization can no longer be set like that, whereas variables affecting autopair's running behaviour can (and should).

Notice, to this effect, that autopair-dont-activate is only available on emacs versions < 24. In emacs versions > 24 the recommended way is to add (autopair-mode -1) to the major mode's hook. And even that means that autopair-mode will be active for a short while and then shut down.

So to summarize, I think that no more variables should be made defcustom. Maybe we could come up with additional "*-alist" versions of those variables that are defcustoms and behave the way you suggest, but do you think it's worth it?

If you're serious about improving autopair, study the code and have a look at the pending issues list. I can make you a committer and possibly maintainer to the repo if you're interested in keeping its spirit of simplicity.

João

On Sat, Oct 5, 2013 at 8:50 PM, Ryan Thompson notifications@github.comwrote:

I've addressed the low-hanging fruit in a97d6b7https://github.com/capitaomorte/autopair/commit/a97d6b7. Tricker ones are the buffer-local variables autopair-dont-activate, autopair-extra-pairs, and autopair-dont-pair. If I understand correctly, these are intended to be set in major mode hooks to appropriate values for a given major mode. Thus obviously a single global customize value is not appropriate for these. Instead, these variables could be change to be alists where keys are major mode names and values are the current values, and then after-change-major-mode-hook could be used to set the buffer-local versions accordingly.

— Reply to this email directly or view it on GitHubhttps://github.com/capitaomorte/autopair/issues/24#issuecomment-25755722 .

João Távora

DarwinAwardWinner commented 11 years ago

One thing that might be worth it is an autopair-conflicting-major-modes custom variable that just lists major modes in which autopair should not be enabled, even if autopair-global-mode is on.

As for helping improve autopair, I'll be happy to help out where I can, but at this point I really don't understand how it works, and I can' devote a huge amount of time to it, so I certainly wouldn't feel comfortable as a maintainer.

joaotavora commented 11 years ago

OK