Matthieu Moy | 3 Oct 15:17
Picon
Picon
Favicon

Re: dvc-add-log-entry vs dvc-partner-buffer

Stephen Leake <stephen_leake <at> stephe-leake.org> 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
(Continue reading)

Stephen Leake | 11 Oct 18:07
Favicon

Re: dvc-add-log-entry vs dvc-partner-buffer

Matthieu Moy <Matthieu.Moy <at> imag.fr> writes:

> Stephen Leake <stephen_leake <at> stephe-leake.org> 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.

Yes.

I've committed a change that does this.

It needs another parameter to dvc-log-edit, indicating whether this
invocation is expecting to fully initialize the buffer or not.

>> 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).

Right. My change does not yet address this.

> I'm not opposed to having a M-x
(Continue reading)

Matthieu Moy | 12 Oct 08:36
Picon
Picon
Favicon

Re: dvc-add-log-entry vs dvc-partner-buffer

Stephen Leake <stephen_leake <at> stephe-leake.org> writes:

> But then what should dvc-log-edit do when called from a source buffer? I'm
> tempted to say "abort with a nice message", but I'm not clear what you prefer.
>
> Ah; you should be able to use dvc-log-edit in the way I'm using
> dvc-add-log-entry.

Yes, that's what I've been trying to say for some time. dvc-log-edit
and dvc-add-log-entry are basically the same, with dvc-add-log-entry
inserting something after having switched to the log-edit buffer.

>> M-x bzr-status RET
>> M-x xmtn-status RET
>
> Almost; it would have to be:
>
> (bzr-status "/Gnu/dvc-fileinfo/")
> (xmtn-status "/Gnu/dvc-fileinfo/")

Yes, but that's orthogonal to the issue of choosing the back-end.

> But I'd rather not, because at some point more functionality might be
> moved into the dvc front-end, and then I'd lose that.

No, you wouldn't.

The _point_ of `define-dvc-unified-command' is to create a dvc-*
function that only calls <back-end>-*.

(Continue reading)

Stephen Leake | 14 Oct 16:53
Favicon

Re: dvc-add-log-entry vs dvc-partner-buffer

Matthieu Moy <Matthieu.Moy <at> imag.fr> writes:

> The _point_ of `define-dvc-unified-command' is to create a dvc-*
> function that only calls <back-end>-*.

Well, ok.

But we've been moving away from that; see dvc-add-files,
dvc-revert-files, dvc-remove-files, and now dvc-log-edit. 

These all have moved some of the user interface code out of the
back-end into the front-end, with the idea of making the DVC UI more
consistent across back-ends.

> _If_ we decide to implement dvc-status another way, with some DVC code
> before doing the dispatching, then we have at least two options:
>
> * stop using `define-dvc-unified-command', and do it the other way
>   around: implement dvc-status, and use `dvc-back-end-wrappers' for
>   that.
>
> * have dvc-status call bzr-dvc-status, which in turn would call
>   dvc-status-internal, which itself is the implementation.

Or, as above, refactor what is done in the front- and back-end
functions with the same name.

If we want to keep functions defined by define-dvc-unified-command as
very thin dispatchers, then we need another convention for the common
DVC front-end UI functions.
(Continue reading)

Matthieu Moy | 14 Oct 17:26
Picon
Picon
Favicon

Re: dvc-add-log-entry vs dvc-partner-buffer

Stephen Leake <stephen_leake <at> stephe-leake.org> writes:

> Matthieu Moy <Matthieu.Moy <at> imag.fr> writes:
>
>> The _point_ of `define-dvc-unified-command' is to create a dvc-*
>> function that only calls <back-end>-*.
>
> Well, ok.
>
> But we've been moving away from that; see dvc-add-files,
> dvc-revert-files, dvc-remove-files, and now dvc-log-edit. 

You probably didn't look at my latest commits. This was a problem in
the existing code base, this problem is a solved one now.

> These all have moved some of the user interface code out of the
> back-end into the front-end, with the idea of making the DVC UI more
> consistent across back-ends.

Which is good, yes. The functions you mention above just lacked a way
to be called for a given back-end, they have one now.

>> _If_ we decide to implement dvc-status another way, with some DVC code
>> before doing the dispatching, then we have at least two options:
>>
>> * stop using `define-dvc-unified-command', and do it the other way
>>   around: implement dvc-status, and use `dvc-back-end-wrappers' for
>>   that.
>>
>> * have dvc-status call bzr-dvc-status, which in turn would call
(Continue reading)

Stephen Leake | 11 Oct 18:45
Favicon

Re: dvc-add-log-entry vs dvc-partner-buffer

Stephen Leake <stephen_leake <at> stephe-leake.org> writes:

> Matthieu Moy <Matthieu.Moy <at> imag.fr> writes:
>
>> Stephen Leake <stephen_leake <at> stephe-leake.org> 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.
>
> Yes.
>
> I've committed a change that does this.

Actually, I haven't; I'm having problems updating my web site from
this Internet cafe. So it will have to wait until I get back from
vacation - after the weekend.

--
-- Stephe
Matthieu Moy | 4 Oct 00:27
Picon
Picon
Favicon

Re: dvc-add-log-entry vs dvc-partner-buffer

Matthieu Moy <Matthieu.Moy <at> imag.fr> writes:

> 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.

Just commited.

So, you can now explicitely ask M-x xgit-log-edit RET if you want to
commit in the git repository, even if it's not the default (i.e. not
the most nested tree, or not the first in dvc-select-priority).

It doesn't solve the problem of nested trees contained in each other
using the same back-end, but I don't think DVC will ever support it,
and at least _I_ will not implement it.

Please, test the patch and let me know if it suits you.

Then, you can fix your patch to only look for buffers managed by the
(Continue reading)


Gmane