On Mon, Jul 5, 2010 at 11:09 AM, Nathan Weizenbaum <[email protected]> wrote:
> Gosh, I should probably subscribe to this list.
>
> For my GitHub support, I've actually found relatively few places (so
> far) where I've felt the need to substantially hook into what Magit is
> currently doing. That said, I haven't yet tackled anything that would
> involve adding new sections, modifying existing actions, and so forth.
> Most of the extensions proposed by Yann look good to me on that front.
>
> I'm a little iffy about magit-define-command, though; it seems like
> that use-case is adequately handled by advice. Is there any reason to
> use a custom macro rather than advice in this case?
Hi,
thanks for your comments. Your GitHub support looks very nice, I shall
have a deeper look :-)
About advising functions, I agree this is the way to go when you stay
external to the code.
Still, it can quickly become a maintenance nightmare, when the
underlying functions evolve out of control. Since this patch intends
to go upstream, I think it'd be more robust to rely on the proper set
of hooks.
See relevant section from the Elisp manual (a.k.a. Bible ;-))
*Usage note:* Advising a function can cause confusion in debugging,
since people who debug calls to the original function may not notice
that it has been modified with advice. Therefore, if you have the
possibility to change the code of that function (or ask someone to do
so) to run a hook, please solve the problem that way. Advice should be
reserved for the cases where you cannot get the function changed.
> One thing that has been a bit painful for me, and that I could foresee
> becoming a bigger problem in the future, is the keybinding namespace.
> Magit has is pretty well saturated; for my GitHub stuff, I'm using the
> apostrophe as a prefix character, but there are only so many of those
> that are currently unused. It would be nice if Magit supplied some
> guideline as to appropriate keybindings for extensions to use.
Totally agree. I was more or less considering proposing to enforce
extension bindings into dedicated maps (maybe with "#" or "&" prefix).
Generally speaking I think some reworking of the global map will be unavoidable.
Yann.
