Stefan Reichör <[EMAIL PROTECTED]> writes:

> Daniel Dehennin <[EMAIL PROTECTED]> writes:
>
>> After a merge I use to hit C to add the "merge from XXXX" log message
>> and commit.
>>
>> With the latest 'mainline' dvc I got this error message:
>>
>> dvc-log-edit: Must have a DVC diff or status buffer before calling
>> dvc-log-edit
>
> That is a problem that I also noticed.
> Stephen Leake introduced this check.

Right. I did not consider starting a commit from a bookmark buffer.

> I also use this flow from the bookmarkbuffer:
>
> M x       - to merge
> c         - start a commit
> C-c C-RET - Insert the merged from text
> C-c C-d   - Show the diff
> add log message
> C-c C-c   - commit
>
>
> Now we have to open a status buffer or a diff buffer to start a
> commit

This is required to set `dvc-partner-buffer' to the diff or status
buffer (more below).

> One way to solve this problem would be to show the diff first 

Since you are opening a diff buffer anyway, it seems the only thing
you lose is "Insert the merged from text".

C-c C-RET is not defined for me, in either the bookmark or log-edit
buffers. What function does it invoke?

But you might also want to commit without showing the diff, if you
know there are no local changes.

> and add a a way to open the log-edit buffer after the diff is shown.
> Any suggestions about the best way to do this?

>From a dvc-diff buffer, 'c' opens the log-edit buffer. But I'm not
sure I understand your question.

Perhaps you mean 'how do I bind C-c C-d in the bookmarks buffer to
show the diff'. That's similar to log-edit:

(defun dvc-bookmarks-diff ()
  (interactive)
  (let ((local-tree (dvc-bookmarks-current-value 'local-tree)))
    (if local-tree
        (let ((default-directory local-tree))
          (dvc-diff))
      (message "No local-tree defined for this bookmark entry."))))

> Or would it be possible to disable the check when called from the
> bookmark buffer? What would be the drawback? I used it before
> without problems.

One issue is setting dvc-buffer-current-active-dvc in the log-edit
buffer; a bookmark buffer can easily support several different
back-ends. That can be fixed by let-binding
dvc-temp-current-active-dvc in dvc-bookmarks-log-edit. I'm not clear
if the bookmark data structure stores the dvc; dvc-bookmarks-log-edit
could call dvc-current-active-dvc after binding default-directory.

A more important issue is that <dvc>-log-edit-done calls
dvc-current-file-list in the dvc-partner-buffer to get the list of
files to commit. 

Currently, dvc-current-file-list recognizes dired-mode and
dvc-diff-mode, both of which support marking files. In those modes, if
no files are marked, it returns either nil or the list of all files,
depending on the `selection-mode' argument.

For other modes, it returns nil or the result of
dvc-get-file-info-at-point. dvc-get-file-info-at-point defaults to
buffer-file-name.

All back-ends currently pass 'nil-if-none-marked for `selection-mode'
when called from <dvc>-dvc-log-edit-done; that means
dvc-current-file-list will return nil for an unrecognized mode. But I
don't think we should rely on that for future back-ends, or future
additions to current back-ends. I suppose we could just document that
requirement.

dvc-log-insert-commit-file-list also uses dvc-current-file-list, via
<dvc>-dvc-files-to-commit. That passes 'all-if-none-marked. Perhaps
all back-end implementations of dvc-log-edit-done should also be using
dvc-files-to-commit? On the other hand, currently no back-ends
override this function; perhaps dvc-files-to-commit should be deleted.

One option is to add dvc-bookmark-mode to dvc-current-file-list,
returning nil for 'nil-if-none-marked, throwing an error for
'all-if-none-marked. Then have dvc-log-edit allow dvc-bookmark buffers
as dvc-partner-buffer.

Another option is to have dvc-log-edit set dvc-partner-buffer to nil
for unrecognized modes, and enhance all backends to handle that.
Currently xmtn doesn't, and tla doesn't use dvc-partner-buffer at all;
the other back-ends handle a null dvc-partner-buffer. This would also
break dvc-log-insert-commit-file-list. I don't think that breaks
anything else at the moment, but I'd rather be able to rely on a
well-defined dvc-partner-buffer for future features.

Another issue is the support I just added for excluded files, in the
experiment.fileinfo branch. It assumes dvc-partner-buffer has a
dvc-fileinfo-ewoc, containing a list of excluded files. This could be
fixed by defining a function dvc-excluded-file-list that works like
dvc-current-file-list. 

We should fix dvc-log-edit to allow all modes that
dvc-current-file-list supports, since that is the true requirement.
I've added comments in dvc-log-edit and dvc-current-file-list saying
that.

However, dvc-get-matching-buffers won't find dired buffers; is there
another way to find them?

Summary:

There is a reasonable use case for commit from a bookmark buffer after
a merge.

If there are local changes, opening a diff or status buffer first is
the right solution, and requires only the addition of
'dvc-bookmark-diff' and 'dvc-bookmark-status'.

If there are no local changes, it would be nice to not have to open a
diff or status buffer, since that can be time-consuming. In this case,
either dvc-bookmark-mode should be added to dvc-current-file-list as
described above, or dvc-log-edit should set dvc-partner-buffer to
nil, and xmtn fixed to allow that.

I prefer _not_ allowing dvc-partner-buffer to be nil.

-- 
-- Stephe

_______________________________________________
Dvc-dev mailing list
[email protected]
https://mail.gna.org/listinfo/dvc-dev

Reply via email to