Stephen Leake <[EMAIL PROTECTED]> writes:
> Matthieu Moy <[EMAIL PROTECTED]> writes:
>
>>>> 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.
>
> True. But in this case, it's clear that you are abusing the mechanism;
> hooks are intended for minor tweaks, not wholesale overrding.
>
> While a dispatching dvc function _is_ intended for wholesale
> overriding, in most cases.
Yes, but in both cases, mis-using the dispatching can break DVC.
>>> 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
>
> I see; you added a new top-level menu, rather than a child of the
> current dvc-diff-mode-menu.
>
> That's a choice.
On that particular point, it's just because I didn't manage to get it
right, but it'd be better to have some additional items in the same
menu. That's not totally trivial to do, since you want to add the
items for that particular instance of the menu, and not for the same
menu in other back-ends (i.e. The DVC-Diff menu a *bzr-status*
buffer).
>> * Set the keymap yourself
>
> This is a side effect of using define-derived-mode; in a hook, you can
> easily add to the current keymap.
That's one of the _points_ using it.
> I agree that define-derived-mode makes for very clean code in this
> case.
>
> I have other code that adds to the Ada-mode menu; it looks like this:
>
> (define-key-after (lookup-key ada-mode-map [menu-bar Ada Project]) [reload]
> '("Reload" . ada-prj-reload-project-file) 'Load)
>
> much harder to understand.
And incorrect AAUI. This would mutate dvc-diff-mode-map, and the
changes would apply to other back-ends too.
> So maybe if we have a policy that says:
>
> "<back-end>-dvc-diff-mode" should be derived from dvc-diff-mode
> (via define-derived-mode), and only extend the menu and keymap
> (see xgit.el for a good example).
Why not. But I can also imagine a mode for a totally weird (imaginary
as of now) back-end that would actually need to totally override
dvc-diff-mode. But, well, let's delay this case for the day we find
such weird back-end ;-).
--
Matthieu
_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev
