Christian Ohler <[EMAIL PROTECTED]> writes:
>> Even easier, you can just mark the buffer as OK to commit from after
>> creating it, that's one local variable.
>
> One local variable, but N commands times M back-ends that need to check
> it. The problem affects all workspace operations, not just commit, and
> workspace operations are currently mostly defined as "unified" commands
> that don't run any shared code before the back-end-specific code, so
> there is nowhere to place a common check.
> This would definitely be an added maintenance burden.
First of all, I don't see why you see the possibility to commit from a
wrong diff buffer as a real problem. OK, it doesn't really make sense,
but does it harm? We've been developing DVC, and its predecessor Xtla
for 3 or 4 years now, and we never got a complain about this
possibility. Has it ever been a problem for you? I don't mean a safety
check would not be interesting, but it really doesn't seem to be a
priority to me.
Now, suppose I want to view the diff between two revisions, A and B.
With your proposal, I can view the diff, but I have to forget about
the summary at the top, *and* I have to forget about having a summary
in a separate buffer too, because this separate buffer would have to
be a status buffer, and we're back to the original problem : you'd
want to disable commit from this buffer. Otherwise, we'd need a 3rd
mode, for summary-which-isn't-status.
> We could also restructure the command definitions to have shared code
> handle the check of this variable, but that would be a nontrivial
> refactoring.
That *would* be a trivial refactoring. Adding a wrapper function for
the unified commands would take ~5 lines of code for each of the
functions.
(defun dvc-safe-whatever (...)
(if (dvc-commit-is-possible)
(dvc-whatever ...)
(error "Command dissallowed here")))
(I realize that a clever lisp macro could even refactor it more
efficiently if the number of commands to check is big ;-) ).
>> BTW, commiting from a diff buffer which isn't against the head can be
>> meaningfull too. For example, "git commit --amend" commits against
>> the ancestor of the head, and discards the current head (I don't
>> think we have a good interface for this in DVC).
>
> Whether the user wants --amend is unrelated to which diff he's looking
> at. It's probably relatively common to look at the diff relative to the
> HEAD while still wanting --amend.
In the best case, you'd look at both: HEAD to check that you are
actually commiting a fix for the previous commit, and against its
ancestor (HEAD^ in the git syntax) to check that the combination of
both makes sense. It depends on the use case. I also sometimes re-work
a patch, and don't want to record the intermediate steps. Then, I only
look at the diff against HEAD^.
And commiting from a diff against the last but one revision is indeed
a common use-case: I often used it (that's when I wrote
tla-changes-last-revision) to fix or comment other people's work after
merging it. What do you propose for this senario with back-ends not
supporting "status" against the ancestor of HEAD (bzr for example)?
I'm repeating myself, but you're trying to add a safety check for
something which is a non-problem, and this would remove many
interesting possibilities from DVC.
--
Matthieu
_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev
