Stephen Leake <[EMAIL PROTECTED]> writes:
> Matthieu Moy <[EMAIL PROTECTED]> writes:
>
>> Stephen Leake <[EMAIL PROTECTED]> writes:
>>
>>> Thus it would seem reasonable that _all_ dispatching functions that
>>> are interactive should be in dvc-back-end-wrappers.
>>
>> A function which is defined with `define-dvc-unified-command' doesn't
>> need that, since by definition, there is already a <back-end>-whatever
>> command for each back-end.
>
> Well, no; the back-end commands defined by define-dvc-unified-command
> all have "dvc" in the name; the wrappers generated by
> dvc-back-end-wrappers do not.
Hmm, right, but indeed, the <back-end>-whatever is the command, and we
define an alias <back-end>-dvc-whatever to have it callable by the
define-dvc-unified-command'ed function.
> For example, bzr has both bzr-dvc-log-edit-done and bzr-lot-edit-done
> (aliased to each other), but xmtn has only xmtn-dvc-log-edit-done.
>
> So that is confusing.
Yes, we can clarify this.
> In addition, calling the back-end function directly is _not_ the same
> as calling the unified function; in many cases the defaults for the
> interactive arguments are different. So using wrappers generated via
> dvc-back-end-wrappers gives more consistent behavior.
True, I didn't notice that. Well, normally, the (interactive ...)
thing would be the same, but that's just programmer's discipline, and
we can do better here.
One way to fix that would be to remove the (interactive ...) part of
define-dvc-unified-command, and use `call-interactively' to take the
(interactive ...) statement of the target function.
Another would be to keep define-dvc-unified-command as it is, but
instead of defining <back-end>-whatever, and aliasing it to
<back-end>-dvc-whatever, we would define <back-end>-dvc-whatever, and
use the dvc-back-end-wrappers to define <back-end>-dvc-whatever.
>> `dvc-back-end-wrappers' is only needed when an interactive function is
>> defined with (defun dvc-<command> ...), with dvc-<command> actually
>> doing some dispatching later on.
>
> And only when the corresponding function is _not_ already defined by
> the back-end.
That's two different questions. It's needed whether the function is
defined or not in the back-end. It's nuisive (name clash) when the
function is defined in the back-end, so the solution is to remove the
version defined in the back-ends.
> not in dvc-back-end-wrappers:
>
> "tree-root" - from dvc-tree-root
> <back-end>-dvc- defined in baz, bzr, tla, xdarcs, xgit, xhg
> <back-end>- defined in baz, bzr, tla, xdarcs, xgit, xhg, xmtn
This one is really an exception, it's more or less part of the
dispatching mechanism.
> note that <back-end>-dvc-tree-root is not needed; we should delete
> these.
That's probably true.
> "dvc-diff" - from dvc-diff
> <back-end>-dvc- defined in baz, bzr, tla, xdarcs, xgit, xhg, xmtn
> <back-end>- defined in baz, bzr, xdarcs, xgit, xhg
> "dvc-status" - dvc-status
> <back-end>-dvc- defined in baz, bzr, tla, xdarcs, xgit, xhg, xmtn
> <back-end>- defined in bzr, xdarcs, xgit, xhg
> "dvc-log" - dvc-log
> <back-end>-dvc- defined in baz, bzr, tla, xgit, xhg, xmtn
> <back-end>- defined in bzr, xgit, xhg
> "dvc-command-version" - dvc-command-version
> <back-end>-dvc- defined in baz, bzr, tla, xdarcs, xgit, xhg, xmtn
> <back-end>- defined in baz, bzr, tla, xdarcs, xgit, xhg
>
> The gaps in the above table indicate missing back-end-specific
> functions; for example, the user cannot call baz-log, tla-log,
> xmtn-log; they have to call baz-dvc-log etc. They cannot tell which to
> call except by experimenting.
About baz and tla, that was more or less intentional, since the
corresponding function is "baz|tla revisions", and M-x
baz-revisions RET is the historical names for these functions. So, we
plugged that into DVC with tla|baz-dvc-log, but never defined
yet-another-alies as tla-log.
'doesn't mean I'm opposed to defining the alias, but the code also
makes sense it is. I don't know about xmtn.
> You posted a fix for this in rev 277, which I haven't gotten yet. It
> removes the "<back-end>-" definitions, leaving only the wrappers. But
> it seems very fragile (as you discovered on your first attempt :);
Well, that's the old good problem of name clashes. We're using lisp,
so, name clash is a problem. I did a mistake and created two different
functions with the same name, and obviously, it breaks, but I don't
think that's specific to the wrappers.
What other solution do you propose?
> lots of other similar functions are defined by aliases that define
> "<back-end>-dvc-<foo>" as "<back-end>-<foo>", and if anyone introduces
> a new interactive function, it is not clear what pattern they should
> follow. I guess that can be covered by documentation in DVC-API.
Yes, we should clarify this a bit here, and document it.
(sorry, lack of time, bla bla bla, 'won't have time to invest before ~
monday or tuesday at best).
> In addition, dvc-back-end-wrappers contains these two entries:
>
> ("ignore-file-extensions" (file-list))
> ("ignore-file-extensions-in-dir" (file-list)))
>
> There is a function dvc-ignore-file-extensions, which does some
> front-end stuff and then dispatches to the back-end function
> <backend>-dvc-backend-ignore-file-extensions. And similarly for
> "-in-dir". So these wrappers make sense, but don't follow the "normal"
> dvc naming conventions
I find that perfectly fine. It fits with the usual scheme "target of
dispatchers are <back-end>-dvc-whatever" (as we noticed already in the
thread, it's the widely used scheme, but not enforced, not always
followed, and not well documented), and the "M-x dvc-whatever RET is
user-callable", and with the wrappers, it allows the "M-x
<back-end>-whatever RET" is user-callable too.
> (I added them before I was really familiar with the naming
> conventions). On the other hand, if this alternate naming convention
> was used uniformly, there would be no possibility of
> dvc-back-end-wrappers breaking something.
Agreed.
--
Matthieu
_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev
