Re: vc-*-root finctions

[Dan Nicolaescu]

Thien-Thi Nguyen <address@hidden> writes:

  > () Stefan Monnier <address@hidden>
  > Please see the `vc-status-mode' docstring in the updated code below.
  > This time i include a full patch for anyone interested in trying
  > it out.  In terms of user experience, this means that doing:
  > M-x vc-status RET ~/build/MISC/ferm/examples RET
  > 
  > shows a buffer where the first line reads:
  > Directory: ~/build/MISC/ferm/examples/
  > 
  > and "ferm" (the project's "root" directory) is a button, which
  > means (for me) underlined and clickable, whose action is to do
  > `(vc-status "~/build/MISC/ferm")'.
  > 
  > If this explanation makes sense to you and the docstring doesn't,
  > could you suggest another wording?

Allowing the user to look at the status from the root of the VC tree
seems like a good idea.  But is this the right UI for it?  It does not
seem very intuitive to me...

  > --- 167,186 ----
  >   ;;   in older versions this method was not required to recurse into
  >   ;;   subdirectories.)
  >   ;;
  > ! ;; - dir-status (&optional kickp)
  >   ;;
  >   ;;   This function is used by vc-status, a replacement for vc-dired.
  >   ;;   vc-status is still under development, and is NOT feature
  >   ;;   complete.  As such, the requirements for this function might
  > ! ;;   change.  This is a replacement for dir-state.
  > ! ;;
  > ! ;;   Produce RESULT: a list of conses of the form (file . vc-state)
  > ! ;;   for the files in DIR.  This function is called twice, the first
                                                ^^^^^^^^^^^^^^^^
Can you please explain why? 

  > ! ;;   time with KICKP t, the second time, with KICKP nil.  In both calls,

What's the meaning of KICKP ? 

  > ! ;;   the current buffer is a scratch buffer with `default-directory'
  > ! ;;   set appropriately.  If the backend workings are asynchronous, it
  > ! ;;   must use the current buffer as its process buffer.  The return
  > ! ;;   value of the second call is RESULT.

Why bother making the backend synchronous?
The current API can be used by a synchronous backend too.  It just needs
to call the UPDATE-FUNCTION when done processing.

- (defun vc-status-headers (backend dir)
-   (concat
-    (format "VC backend : %s\n" backend)
-    "Repository : The repository goes here\n"
-    (format "Working dir: %s\n" dir)))
- 

Why remove this? 


  >   ;;;###autoload
  >   (defun vc-status (dir)
  > !   "Show the VC status for DIR in its own buffer.
  > ! Reuse an existing buffer if possible, otherwise create a new one
  > ! and place it in `vc-status-mode'.  Lastly, run `vc-status-refresh'."
  >     (interactive "DVC status for directory: ")
  > !   (setq dir (file-name-as-directory dir))
  > !   (let ((ls (buffer-list))
  > !         buf)
  > !     (while (and ls (not buf))
  > !       (with-current-buffer (car ls)
  > !         (when (and vc-status (string= dir default-directory))
  > !           (setq buf (car ls)))
  > !         (setq ls (cdr ls))))
  > !     (unless buf
  > !       (set-buffer (setq buf (get-buffer-create
  > !                              (generate-new-buffer-name
  > !                               (file-name-nondirectory
  > !                                (directory-file-name dir))))))
  > !       (setq default-directory dir)
  > !       (vc-status-mode))
  > !     (switch-to-buffer buf))
  > !   (vc-status-refresh))
  >   
  >   (defvar vc-status-mode-map
  >     (let ((map (make-keymap)))
  > ***************
  > *** 2777,2818 ****
  >   
  >   (defun vc-status-mode ()
  >     "Major mode for VC status.
  >   \\{vc-status-mode-map}"
  > !   (setq mode-name "*VC Status*")
  > !   (setq major-mode 'vc-status-mode)
  > !   (setq buffer-read-only t)
  > !   (use-local-map vc-status-mode-map)
  > !   (let ((buffer-read-only nil)
  > !   (backend (vc-responsible-backend default-directory))
  > !   entries)
  > !     (erase-buffer)
  >       (set (make-local-variable 'vc-status)
  > !    (ewoc-create #'vc-status-printer
  > !                 (vc-status-headers backend default-directory)))
  > !     (vc-status-refresh)))
  >   
  >   (put 'vc-status-mode 'mode-class 'special)
  >   
  > - (defun vc-update-vc-status-buffer (entries buffer)
  > -   (with-current-buffer buffer
  > -     (dolist (entry entries)
  > -       (ewoc-enter-last vc-status
  > -                  (vc-status-create-fileinfo (cdr entry) (car entry))))
  > -     (ewoc-goto-node vc-status (ewoc-nth vc-status 0))))
  > - 
  >   (defun vc-status-refresh ()
  > !   "Refresh the contents of the VC status buffer."
  >     (interactive)
  > !   ;; This is not very efficient; ewoc could use a new function here.
  > !   (ewoc-filter vc-status (lambda (node) nil))
  > !   (let ((backend (vc-responsible-backend default-directory)))
  > !     ;; Call the dir-status backend function. dir-status is supposed to
  > !     ;; be asynchronous.  It should compute the results and call the
  > !     ;; function passed as a an arg to update the vc-status buffer with
  > !     ;; the results.
  > !     (vc-call-backend
  > !      backend 'dir-status default-directory
  > !      #'vc-update-vc-status-buffer (current-buffer))))
  >   
  >   (defun vc-status-next-line (arg)
  >     "Go to the next line.
  > --- 2790,2887 ----
  >   
  >   (defun vc-status-mode ()
  >     "Major mode for VC status.
  > + Prepare the buffer to begin with the lines:
  > + 
  > + Directory: DEFAULT-DIRECTORY
  > +   Updated: YYYY-MM-DD HH:MM:SS
  > + 
  > + If the `default-directory' is under the project's \"root\"
  > + directory, make its root component a button whose action is
  > + to run command `vc-status' there.
  > + 
  > + Keys do not self-insert; instead they do different things:
  >   \\{vc-status-mode-map}"
  > !   (buffer-disable-undo)
  > !   (erase-buffer)
  > !   (let* ((backend (vc-responsible-backend default-directory))
  > !          (find-root (vc-find-backend-function backend 'root))
  > !          (root (if find-root
  > !                    (funcall find-root default-directory)
  > !                  default-directory)))
  > !     (setq major-mode 'vc-status-mode)
  > !     (setq mode-name (format "VC-%s Status" backend))
  > !     (insert "Directory: ")
  > !     (if (or (not root) (string= root default-directory))
  > !         (insert root)
  > !       (let* ((root-fn (directory-file-name root))
  > !              (parent (file-name-directory root-fn))
  > !              (leaf (file-name-nondirectory root-fn)))
  > !         (insert parent)
  > !         ;; As of 2007-08-21, loadup.el includes button, so this
  > !         ;; check is just future-proofing; not strictly necessary.
  > !         (if (featurep 'button)
  > !             (insert-text-button
  > !              leaf
  > !              'root root
  > !              'action (lambda (button)
  > !                        (vc-status (button-get button 'root)))
  > !              'follow-link t)
  > !           (insert leaf))
  > !         (insert (substring default-directory (1- (length root))))))
  > !     (insert "\n")
  >       (set (make-local-variable 'vc-status)
  > !          (ewoc-create #'vc-status-printer
  > !                       (format-time-string "  Updated: %F %T\n")))
  > !     (use-local-map vc-status-mode-map)
  > !     (setq buffer-read-only t)))
  >   
  >   (put 'vc-status-mode 'mode-class 'special)
  >   
  >   (defun vc-status-refresh ()
  > !   "Refresh the contents of the VC Status buffer."
  >     (interactive)
  > !   (unless vc-status
  > !     (error "Not in a VC Status buffer"))
  > !   (ewoc-filter vc-status 'ignore)
  > !   (let* ((backend (vc-responsible-backend default-directory))
  > !          (get-status (cond ((vc-find-backend-function backend 
'dir-status))
  > !                            (t (kill-buffer nil)
  > !                               (error "No vc-status support for %s"
  > !                                      backend))))
  > !          (here (current-buffer))
  > !          (scratch (get-buffer-create (format " vc status: %s"
  > !                                              default-directory)))
  > !          notice)
  > !     ;; Call the backend function in two-phase style.  First, kick...
  > !     (with-current-buffer scratch
  > !       (erase-buffer)
  > !       (funcall get-status t))
  > !     ;; Clue in the user if things are working asynchronously.
  > !     (when (setq notice (buffer-local-value 'mode-line-process scratch))
  > !       (setq mode-line-process notice)
  > !       (ewoc-set-hf vc-status
  > !                    (format "  Updated: %s -- %s working...\n"

Is this the time when vc-status was last run?  That might be confusing
for the user, he might think that "Updated:" means the last time he
updated the his sources from the VC system.
  

  > !                            (format-time-string "%F %T")
  > !                            backend)
  > !                    ""))



  > !     (with-current-buffer scratch
  > !       (vc-exec-after
  > !        ;; ... then collect.
  > !        `(let ((entries (,get-status)))
  > !           (when (buffer-live-p ,here)
  > !             (with-current-buffer ,here
  > !               (dolist (entry entries)
  > !                 (ewoc-enter-last vc-status (vc-status-create-fileinfo
  > !                                             (cdr entry) (car entry))))
  > !               (let ((first (ewoc-nth vc-status 0)))
  > !                 (when first
  > !                   (ewoc-goto-node vc-status first)
  > !                   (vc-status-move-to-goal-column))
  > !                 (ewoc-set-hf vc-status
  > !                              (format-time-string "  Updated: %F %T\n")
  > !                              (if first "" "(no entries)"))
  > !                 (setq mode-line-process nil))))
  > !           (kill-buffer nil))))))

This function is much more complicated now, it's not obvious why.  Can
you please explain?