Stephen Leake <[EMAIL PROTECTED]> writes:
> So dvc-add-log-entry can't just call dvc-log-edit to reuse a buffer;
> it has to switch to it only.
Probably it's better to have dvc-log-edit itself reuse the buffer
without setting the partner buffer, so that both log-edit and
add-log-entry benefit of the fix.
>>> Ah. That is a different solution. I don't think it's viable; consider
>>> the case of two parallel workspaces, each controlled by different
>>> back-ends with different repositories. You always commit them at the
>>> same time, and review them in parallel, in two status buffers. Thus
>>> there are two status buffers with marked files, and two log-edit
>>> buffers; which does the commit choose?
>>
>> If the buffers are in sync, you don't care which one to chose.
>
> Since they are commiting to different repositories, it does make a
> difference.
>
> Suppose the workspace for project A is ready to commit, but B still
> has some files to review. You are in the log-edit buffer for A, and
> hit C-c C-c. But dvc-log-edit-done chooses the log-edit buffer for
^^^^^^^^
I suppose you mean "status" here.
> B, and it does the wrong thing.
Then, your point is not that you can choose the wrong buffer, but that
it's bad to synchronize the buffers for your use-case.
> In that case, dvc-log-edit needs to use the same logic as dvc-status
> to figure out the back-end and root; that is, it needs to consistently
> use dvc-current-active-dvc.
Yes, there's no point having different mechanisms to chose the
back-end for different commands.
> And dvc-log-edit needs to set dvc-partner-buffer in a way that
> tells dvc-log-edit-done that it should commit the source buffer only.
> Setting dvc-partner-buffer to the source file in this case makes
> sense, but I don't think any of the current back-ends treat that as
> meaning "commit only this file".
Not AFAIK, and that would be awfully bad to do this silently (one
would really think he's doing a full commit). I'm not opposed to
having a M-x dvc-log-edit-to-commit-just-this-file RET (probably with
a shorter name ;-) ).
> There is also a problem when there are multiple back-ends for a
> workspace. Currently, to select the back-end, I have the following in
> a text file:
>
> (let ((dvc-temp-current-active-dvc 'bzr)) (dvc-status "/Gnu/dvc-fileinfo/"))
> (let ((dvc-temp-current-active-dvc 'xmtn)) (dvc-status "/Gnu/dvc-fileinfo/"))
M-x bzr-status RET
M-x xmtn-status RET
That you can easily bind to the key of your choice.
You don't _have_ to use DVC's unification layer, you just _can_ use
it.
> Perhaps dvc-log-edit could prompt the user if two back-ends are
> detected. I'm not sure if we want to put that into
> dvc-current-active-dvc.
It can be a good idea to add it, if you keep it optional (and disabled
by default), and possibly customizable.
Well, if you have two back-ends for the exact same tree, it's OK for
me to prompt. But for nested trees, the use-case where you version
the nested tree also with the superproject is rare, while actually
nested projects are common (and the norm for people who version their
$HOME).
Ideally, I'd see something like a variable dvc-prompt-dvc with
possible values
* nil => don't prompt, ever. The default.
* t => prompt whenever there's a choice.
* list of strings => list of paths where the user should be prompted.
But once more, that's not limited to log-edit. Don't make dvc-status
behave differently from dvc-log-edit.
The one difference I can see between log-edit and status now is that
there are no <back-end>-log-edit functions that you can call by
yourself. I think a good solution would be to add it. That can be done
automatically from DVC's core, adding a wrapper function like
(defun <back-end>-log-edit ()
(interactive)
(let ((dvc-temp-current-active-dvc '<back-end>)) (dvc-log-edit)))
Then, the normal flow would be to call M-x dvc-log-edit RET (or
keybinding), and users with specific needs can call M-x
<back-end>-log-edit RET explicitely.
I can try to write a patch doing that ~tonight.
Same can be done for dvc-add-log-entry. Actually, in a similar way we
have define-dvc-unified-command, we should have
define-dvc-wrapped-command, that would define the above wrapper for a
given command.
>>> At this time, we don't have to address the issue of dvc-log-edit not
>>> working correctly when called this way.
>>
>> We do. For a simple reason :
>>
>> consistency.
>
> That means we need to do it eventually; not right away.
Right now, we don't have inconsistancy between dvc-log-edit and
dvc-add-log-entry. They behave similarly. If you change one, don't
introduce inconsistancy by not changing the other.
> There are _many_ inconsistencies in the current code! We can't fix
> everything at once.
True, the current code quality of DVC is not as good as it should be.
I don't mean you should fix all of it right now, I know it would take
ages.
>> You're trying to adress multiple problems (dvc-add-log-entry, nested
>> trees management, and the way partial commits are done)
>
> Well, the _problem_ is dvc-add-log-entry; the _context_ is nested
> trees, multiple back-ends, and partial commits. It's the same
> context for all DVC issues.
>
>> in a solution that would only change dvc-log-edit,
>
> No, I'm trying to change only dvc-add-log-entry.
s/log-edit/add-log-entry/ in my sentence above, sorry.
>> and which makes it inconsistant with other commands.
>
> dvc-add-log-entry is currently inconsistent in a typical usage (at
> least for me). I'm trying to make it consistent.
>
> dvc-log-edit is also inconsistent as you point out above, but in a
> rare usage (for both of us; others may disagree that it is rare).
>
> These are two separate problems, and as far as I can tell at the
> moment, they have different, unrelated solutions.
I don't see any difference.
dvc-add-log-entry creates/reuses a log buffer, and inserts something
dvc-log-edit creates/reuses a log buffer, and nothing else.
If you wish to change something wich isn't in the common portion,
fine, but here, you're trying to change something which is precisely
inside the intersection of both.
> Hmm. One variant of the "quick single file commit" case is the user
I didn't talk about "quick single file commit", but about "quick
commit without review".
> Hmm. Or perhaps the intelligent prompting should be moved into
> dvc-log-edit. That would make the solutions overlap.
That's probably the way to go, yes (see above).
> Things get complicated when you try to address more than one problem
> at a time :).
I'm not.
Your patch does something that DVC never do AFAIK, which is to have
interference between buffers belonging to one back-end and projects
managed by another back-end (as I pointed out, having a *bzr-log*
buffer for ~/some/bzr/project prevents me from using add-log-entry in
~/another/project/managed/by/git), and the rationale you gave was to
solve your nested tree problem.
I'm _precisely_ insisting on solving the add-log-entry problem without
interfering the nested-tree one. *If* you need a solution for
nested-trees, we should implement one.
> I'd like to just fix dvc-add-log-entry. I'll try to write the
> prompting code in a nicely factored way, so it can be reused or moved
> if we decide to do that for these other issues.
>
>> dvc-log-edit is just the same as dvc-add-log-entry, except that
>> dvc-add-log-entry inserts something in the log buffer, while
>> dvc-log-edit doesn't.
>
> Actually, xmtn-dvc-log-edit inserts the entire commit comment, which
> is also a problem for dvc-add-log-entry.
I didn't really try monotone (yet ;-) ), but my feeling is that xmtn
does not use much DVC, and is mostly an independant interface, with a
few hooks into DVC. That's a pity IMO, since there seem to be a lot of
things in xmtn that could benefit to other back-ends.
I have no time to dig further, but a quick look at `xmtn-dvc-log-edit'
doesn't show many monotone-specific stuff.
--
Matthieu
_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev
