Stephen Leake <[EMAIL PROTECTED]> writes:
> Did you try adding something to dvc-diff-mode-hook? That would be much
> simpler. And that's what mode hooks are for, in the general Emacs
> design.
At some point in time, I had such kind of thing. That's harder than it
seems to get working correctly, since you quickly end up /overriding/
keymaps and menus instead of /extending/ them. More below.
>> I think that's a reasonable example of a way to make a back-end
>> different from the other, while still reusing all of DVC.
>
> I disagree; overriding the entire mode function is too heavy-handed.
> There is nothing that prevents the backend from using an entirely new
> function, not derived from dvc-diff-mode or diff-mode.
How is this different from your proposals? Just try:
(add-hook 'dvc-diff-mode-hook (lambda () (gnus-summary-mode)))
and you've turned your diff buffer into a Gnus Summary buffer. That's
lisp, that's life. Nothing prevents you from turning a coffee-machine
into a grass-cutter, nothing prevents you from shooting yourself in
the foot.
> In your case, all you did in the overridden function is add more menu
> entries; the mode hook is a better solution for that.
If you do that cleanly with a hook, you'll have to at least:
* Add the menu to the menubar yourself
* Set the keymap yourself
As a reminder, the _whole_ code managing the xgit-diff-mode is this:
(defvar xgit-diff-mode-map
(let ((map (make-sparse-keymap)))
(define-key map [?A] 'xgit-status-add-u)
(define-key map [?R] 'xgit-status-reset-mixed)
map))
(easy-menu-define xgit-diff-mode-menu xgit-diff-mode-map
"`Git specific changes' menu."
`("GIT-Diff"
["Re-add modified files (add -u)" xgit-status-add-u t]
["Reset index (reset --mixed)" xgit-status-reset-mixed t]
))
(define-derived-mode xgit-diff-mode dvc-diff-mode "xgit-diff"
"Mode redefining a few commands for diff."
)
(16 lines including the blanks)
In addition to the above, this also defines an xgit-mode-hook, which
isn't strictly necessary, but that the user may want to have for
customization (we had a discussion about back-end specific hooks a few
days ago).
> But if we allow the entire mode function to be overridden, it could do
> something entirely different, which would be very confusing for new users.
Yes, you _can_ shoot yourself in the foot.
I'd agree to go for another solution if you find a way to prevent
people from doing that, other than rewritting Emacs in Ada or OCaml.
But I don't think that's going to happen.
> And when someone improves dvc-diff-mode, the back-ends that totally
> override it will not benefit.
Yes, but that also applies to your solution.
If you think about it, your solution is to
* Run the code of dvc-diff-mode,
* Run arbitrary code afterwards.
And my solution is to use define-derived-mode to
* Run the code of dvc-diff-mode,
* Run some non-arbitrary code decided by define-derived-mode (keymap, ...)
* Run arbitrary code afterwards.
;-).
--
Matthieu
_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev
