vc-*-root finctions

vc-*-root finctions

[Stefan Monnier]

Could you explain what the vc-*-root are supposed to do?


        Stefan

Re: vc-*-root finctions

[Thien-Thi Nguyen]

() Stefan Monnier <monnier@iro.umontreal.ca>
() Tue, 19 Feb 2008 17:06:09 -0500

   Could you explain what the vc-*-root are supposed to do?

Would this blurb in vc.el Commentary explain things sufficiently?

;; - root (dir)
;;
;;   Return DIR's "root" directory, that is, a parent directory of
;;   DIR for which the same backend as used for DIR applies.  If no
;;   such parent exists, this function should return DIR.

I am using vc-BACKEND-root for (work-in-progress, see below)
`vc-status-mode' munging.

thi


_________________________________________________________________
(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 that can 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)
        (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)))

Re: vc-*-root finctions

[Stefan Monnier]

> ;; - root (dir)
> ;;
> ;;   Return DIR's "root" directory, that is, a parent directory of
> ;;   DIR for which the same backend as used for DIR applies.  If no
> ;;   such parent exists, this function should return DIR.

This part I inferred, but of course it is necessary (weren't you the
guy complaining harshly about lack of comments in VC's in-development
code ;-)?

> I am using vc-BACKEND-root for (work-in-progress, see below)
> `vc-status-mode' munging.

I don't understand what for (in terms of user-level feature)?


        Stefan

Re: vc-*-root finctions

[Thien-Thi Nguyen]

[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]

() Stefan Monnier <monnier@iro.umontreal.ca>
() Wed, 20 Feb 2008 12:21:55 -0500

   This part I inferred, but of course it is necessary (weren't
   you the guy complaining harshly about lack of comments in VC's
   in-development code ;-)?

I said i find under-commenting obnoxious.  Now i add: everyone is
free to be obnoxious -- makes life interesting.  If these IMHOs
sound like harsh complaining, i must introduce you someday to my
two-year-old...

   > I am using vc-BACKEND-root for (work-in-progress, see below)
   > `vc-status-mode' munging.

   I don't understand what for (in terms of user-level feature)?

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?

thi


________________________________________________________________
one edit
	* vc.el (vc-overview-p): New defsubst.
	(vc-start-entry, vc-finish-logentry): Use it.

another edit
	* vc.el (vc-status-headers): Delete func.
	(vc-status, vc-status-mode, vc-status-refresh): Rewrite.
	(vc-update-vc-status-buffer): Delete func.
	* vc-svn.el (vc-svn-after-dir-status): Incorporate into...
	(vc-svn-dir-status): ...here; update calling sequence.
	* vc-hg.el (vc-hg-after-dir-status): Incorporate into...
	(vc-hg-dir-status): ...here; update calling sequence.
	* vc-git.el (vc-git-after-dir-status): Delete func.
	(vc-git-dir-status): Rewrite.

full munging

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: two edits --]
[-- Type: text/x-diff, Size: 22638 bytes --]

Index: vc-git.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc-git.el,v
retrieving revision 1.38
diff -c -r1.38 vc-git.el
*** vc-git.el	18 Feb 2008 07:46:14 -0000	1.38
--- vc-git.el	20 Feb 2008 18:15:50 -0000
***************
*** 207,258 ****
        ;; fall back to the default VC representation
        (vc-default-dired-state-info 'Git file))))
  
! ;;; vc-dir-status support (EXPERIMENTAL)
! ;;; If vc-directory (which is not half bad under Git, w/ some tweaking)
! ;;; is to go away, vc-dir-status must at least support the same operations.
! ;;; At the moment, vc-dir-status design is still fluid (a kind way to say
! ;;; half-baked, undocumented, and spottily-supported), so the following
! ;;; should be considered likewise ripe for sudden unannounced change.
! ;;; YHBW, HAND.  --ttn
! 
! (defun vc-git-after-dir-status (callback buffer)
!   (sort-regexp-fields t "^. \\(.+\\)$" "\\1" (point-min) (point-max))
!   (let ((map '((?H . cached)
!                (?M . unmerged)
!                (?R . removed)
!                (?C . edited)
!                (?K . removed)           ; ??? "to be killed"
!                (?? . unregistered)))
!         status filename result)
!     (goto-char (point-min))
!     (while (> (point-max) (point))
!       (setq status (string-to-char (buffer-substring (point) (1+ (point))))
!             status (cdr (assq status map))
!             filename (buffer-substring (+ 2 (point)) (line-end-position)))
!       ;; TODO: Add dynamic selection of which status(es) to display, and
!       ;; bubble that up to vc-dir-status.  For now, we consider `cached'
!       ;; to be uninteresting, to mimic vc-directory (somewhat).
!       (unless (eq 'cached status)
          (push (cons filename status) result))
!       (forward-line 1))
!     (funcall callback result buffer)))
! 
! (defun vc-git-dir-status (dir update-function status-buffer)
!   "Return a list of conses (file . state) for DIR."
!   (with-current-buffer
!       (get-buffer-create
!        (expand-file-name " *VC-Git* tmp status" dir))
!     (erase-buffer)
!     (vc-git-command (current-buffer) 'async dir "ls-files" "-t"
!                     "-c"                ; cached
!                     "-d"                ; deleted
!                     "-k"                ; killed
!                     "-m"                ; modified
!                     "-o"                ; others
!                     "--directory"
!                     "--exclude-per-directory=.gitignore")
!     (vc-exec-after
!      `(vc-git-after-dir-status (quote ,update-function) ,status-buffer))))
  
  ;;; STATE-CHANGING FUNCTIONS
  
--- 207,258 ----
        ;; fall back to the default VC representation
        (vc-default-dired-state-info 'Git file))))
  
! (defun vc-git-dir-status (&optional kickp)
!   "Return a list of conses (FILE . STATE) for the default directory."
!   (if kickp
!       ;; Don't do it asynchronously; git is fast and always local.
!       ;; Avoid "-a" so as to be able to distinguish "in index".
!       (call-process "git" nil t nil "status")
!     (let* ((root (vc-git-root default-directory))
!            (sub (file-relative-name default-directory root))
!            ;; If we are not in the project's root dir, discard
!            ;; lines that do not have the relative-dir prefix.
!            (keep-rx (concat "^#\t\\([^:]+\\): +"
!                             (if (member sub '("." "./"))
!                                 ""
!                               (file-name-as-directory sub))))
!            (pair-rx (concat keep-rx "\\(.+\\)$"))
!            status filename result)
!       (goto-char (point-min))
!       ;; Encode "in index" in the state; eg: `modified' vs `modified/in'.
!       (when (search-forward "\n# Changes to be committed:\n" nil t)
!         (search-forward "#\t")
!         (forward-char -2)
!         (while (looking-at "#\t[^:]+\\(:\\)")
!           (replace-match "/in:" t t nil 1)
!           (forward-line 1)))
!       (when (search-forward "\n# Untracked files:\n" nil t)
!         (while (re-search-forward "^#\t" nil t)
!           (insert "untracked: ")))
!       (keep-lines keep-rx (point-min) (point-max))
!       ;; This sorting is purely cosmetic.  We will probably remove it a
!       ;; little further down the road, when VC Status learns to manage
!       ;; total ordering and all that jazz.  --ttn
!       (sort-regexp-fields t pair-rx "\\2" (point-min) (point-max))
!       (goto-char (point-min))
!       (while (re-search-forward pair-rx nil t)
!         (setq status (match-string 1)
!               status (if (string-match "new file" status)
!                          (replace-match "new" t t status)
!                        status)
!               status (intern status)
!               filename (match-string 2))
!         (when (memq status '(renamed renamed/in copied copied/in))
!           ;; Discard first name: "ONE -> TWO" becomes "TWO".
!           (setq filename (substring filename
!                                     (+ 4 (string-match " -> " filename)))))
          (push (cons filename status) result))
!       result)))
  
  ;;; STATE-CHANGING FUNCTIONS
  
Index: vc-hg.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc-hg.el,v
retrieving revision 1.50
diff -c -r1.50 vc-hg.el
*** vc-hg.el	20 Feb 2008 15:21:55 -0000	1.50
--- vc-hg.el	20 Feb 2008 18:15:50 -0000
***************
*** 483,524 ****
  
  (define-derived-mode vc-hg-incoming-mode vc-hg-log-view-mode "Hg-Incoming")
  
! ;; XXX Experimental function for the vc-dired replacement.
! (defun vc-hg-after-dir-status (update-function buff)
!   (let ((status-char nil)
! 	(file nil)
! 	(translation '((?= . up-to-date)
! 		       (?C . up-to-date)
! 		       (?A . added)
! 		       (?R . removed)
! 		       (?M . edited)
! 		       (?I . ignored)
! 		       (?! . deleted)
! 		       (?? . unregistered)))
! 	(translated nil)
! 	  (result nil))
        (goto-char (point-min))
        (while (not (eobp))
! 	(setq status-char (char-after))
! 	(setq file
! 	      (buffer-substring-no-properties (+ (point) 2)
! 					      (line-end-position)))
! 	(setq translated (assoc status-char translation))
! 	(when (and translated (not (eq (cdr translated) 'up-to-date)))
! 	  (push (cons file (cdr translated)) result))
! 	(forward-line))
!       (funcall update-function result buff)))
! 
! ;; XXX Experimental function for the vc-dired replacement.
! (defun vc-hg-dir-status (dir update-function status-buffer)
!   "Return a list of conses (file . state) for DIR."
!   (with-current-buffer
!       (get-buffer-create
!        (expand-file-name " *VC-hg* tmp status" dir))
!     (erase-buffer)
!     (vc-hg-command (current-buffer) 'async dir "status")
!     (vc-exec-after
!      `(vc-hg-after-dir-status (quote ,update-function) ,status-buffer))))
  
  ;; XXX this adds another top level menu, instead figure out how to
  ;; replace the Log-View menu.
--- 483,516 ----
  
  (define-derived-mode vc-hg-incoming-mode vc-hg-log-view-mode "Hg-Incoming")
  
! (defun vc-hg-dir-status (&optional kickp)
!   "Return a list of conses (FILE . STATE) for the default directory."
!   (if kickp
!       ;; TODO: Conditionally synchronous.
!       (vc-hg-command (current-buffer) 'async default-directory "status")
!     (let ((status-char nil)
!           (file nil)
!           (translation '((?= . up-to-date)
!                          (?C . up-to-date)
!                          (?A . added)
!                          (?R . removed)
!                          (?M . edited)
!                          (?I . ignored)
!                          (?! . deleted)
!                          (?? . unregistered)))
!           (translated nil)
!           (result nil))
        (goto-char (point-min))
        (while (not (eobp))
!         (setq status-char (char-after))
!         (setq file
!               (buffer-substring-no-properties (+ (point) 2)
!                                               (line-end-position)))
!         (setq translated (assoc status-char translation))
!         (when (and translated (not (eq (cdr translated) 'up-to-date)))
!           (push (cons file (cdr translated)) result))
!         (forward-line))
!       result)))
  
  ;; XXX this adds another top level menu, instead figure out how to
  ;; replace the Log-View menu.
Index: vc-svn.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc-svn.el,v
retrieving revision 1.69
diff -c -r1.69 vc-svn.el
*** vc-svn.el	20 Feb 2008 10:40:46 -0000	1.69
--- vc-svn.el	20 Feb 2008 18:15:50 -0000
***************
*** 158,191 ****
        (vc-svn-command t 0 nil "status" (if localp "-v" "-u"))
        (vc-svn-parse-status))))
  
! (defun vc-svn-after-dir-status (callback buffer)
!   (let ((state-map '((?A . added)
!                     (?C . edited)
!                     (?D . removed)
!                     (?I . ignored)
!                     (?M . edited)
!                     (?R . removed)
!                     (?? . unregistered)
!                     ;; This is what vc-svn-parse-status does.
!                     (?~ . edited)))
!        result)
!     (goto-char (point-min))
!     (while (re-search-forward "^\\(.\\)..... \\(.*\\)$" nil t)
!       (let ((state (cdr (assq (aref (match-string 1) 0) state-map)))
!            (filename (match-string 2)))
!        (when state
!          (setq result (cons (cons filename state) result)))))
!     (funcall callback result buffer)))
! 
! (defun vc-svn-dir-status (dir callback buffer)
!   "Run 'svn status' for DIR and update BUFFER via CALLBACK.
! CALLBACK is called as (CALLBACK RESULT BUFFER), where
! RESULT is a list of conses (FILE . STATE) for directory DIR."
!   (with-current-buffer (get-buffer-create
!                        (generate-new-buffer-name " *vc svn status*"))
!     (vc-svn-command (current-buffer) 'async nil "status")
!     (vc-exec-after
!      `(vc-svn-after-dir-status (quote ,callback) ,buffer))))
  
  (defun vc-svn-working-revision (file)
    "SVN-specific version of `vc-working-revision'."
--- 158,185 ----
        (vc-svn-command t 0 nil "status" (if localp "-v" "-u"))
        (vc-svn-parse-status))))
  
! (defun vc-svn-dir-status (&optional kickp)
!   "Return a list of conses (FILE . STATE) for the default directory."
!   (if kickp
!       ;; TODO: Conditionally synchronous.
!       (vc-svn-command (current-buffer) 'async nil "status")
!     (let ((state-map '((?A . added)
!                        (?C . edited)
!                        (?D . removed)
!                        (?I . ignored)
!                        (?M . edited)
!                        (?R . removed)
!                        (?? . unregistered)
!                        ;; This is what vc-svn-parse-status does.
!                        (?~ . edited)))
!           result)
!       (goto-char (point-min))
!       (while (re-search-forward "^\\(.\\)..... \\(.*\\)$" nil t)
!         (let ((state (cdr (assq (aref (match-string 1) 0) state-map)))
!               (filename (match-string 2)))
!           (when state
!             (setq result (cons (cons filename state) result)))))
!       result)))
  
  (defun vc-svn-working-revision (file)
    "SVN-specific version of `vc-working-revision'."
Index: vc.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc.el,v
retrieving revision 1.537
diff -c -r1.537 vc.el
*** vc.el	20 Feb 2008 18:11:15 -0000	1.537
--- vc.el	20 Feb 2008 18:15:50 -0000
***************
*** 167,184 ****
  ;;   in older versions this method was not required to recurse into
  ;;   subdirectories.)
  ;;
! ;; - dir-status (dir update-function status-buffer)
  ;;
- ;;   Produce RESULT: a list of conses of the form (file . vc-state)
- ;;   for the files in DIR.  If a command needs to be run to compute
- ;;   this list, it should be run asynchronously.  When RESULT is
- ;;   computed, it should be passed back by doing:
- ;;       (funcall UPDATE-FUNCTION RESULT STATUS-BUFFER)
  ;;   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.
  ;;
  ;; * working-revision (file)
  ;;
--- 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
! ;;   time with KICKP t, the second time, with KICKP nil.  In both calls,
! ;;   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.
  ;;
  ;; * working-revision (file)
  ;;
***************
*** 918,923 ****
--- 920,929 ----
  (defvar vc-dired-mode nil)
  (make-variable-buffer-local 'vc-dired-mode)
  
+ (defsubst vc-overview-p ()
+   "Return non-nil if current buffer is in VC Dired or VC Status mode."
+   (memq major-mode '(vc-dired-mode vc-status-mode)))
+ 
  ;; File property caching
  
  (defun vc-clear-context ()
***************
*** 1794,1802 ****
  \(current one if no file).  AFTER-HOOK specifies the local value
  for `vc-log-after-operation-hook'."
    (let ((parent
!          (if (eq major-mode 'vc-dired-mode)
!              ;; If we are called from VC dired, the parent buffer is
!              ;; the current buffer.
               (current-buffer)
             (if (and files (equal (length files) 1))
                 (get-file-buffer (car files))
--- 1800,1806 ----
  \(current one if no file).  AFTER-HOOK specifies the local value
  for `vc-log-after-operation-hook'."
    (let ((parent
!          (if (vc-overview-p)
               (current-buffer)
             (if (and files (equal (length files) 1))
                 (get-file-buffer (car files))
***************
*** 1934,1940 ****
    ;; Sync parent buffer in case the user modified it while editing the comment.
    ;; But not if it is a vc-dired buffer.
    (with-current-buffer vc-parent-buffer
!     (or vc-dired-mode (vc-buffer-sync)))
    (if (not vc-log-operation)
        (error "No log operation is pending"))
    ;; save the parameters held in buffer-local variables
--- 1938,1944 ----
    ;; Sync parent buffer in case the user modified it while editing the comment.
    ;; But not if it is a vc-dired buffer.
    (with-current-buffer vc-parent-buffer
!     (unless (vc-overview-p) (vc-buffer-sync)))
    (if (not vc-log-operation)
        (error "No log operation is pending"))
    ;; save the parameters held in buffer-local variables
***************
*** 2642,2653 ****
  
  (defvar vc-status nil)
  
- (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)))
- 
  (defun vc-status-printer (fileentry)
    "Pretty print FILEENTRY."
    ;; If you change the layout here, change vc-status-move-to-goal-column.
--- 2646,2651 ----
***************
*** 2673,2684 ****
  
  ;;;###autoload
  (defun vc-status (dir)
!   "Show the VC status for DIR."
    (interactive "DVC status for directory: ")
!   (vc-setup-buffer "*vc-status*")
!   (switch-to-buffer "*vc-status*")
!   (cd dir)
!   (vc-status-mode))
  
  (defvar vc-status-mode-map
    (let ((map (make-keymap)))
--- 2671,2697 ----
  
  ;;;###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"
!                            (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))))))
  
  (defun vc-status-next-line (arg)
    "Go to the next line.

Re: vc-*-root finctions

[Dan Nicolaescu]

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

  > () Stefan Monnier <monnier@iro.umontreal.ca>
  > 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?

Re: vc-*-root finctions

[Thien-Thi Nguyen]

[-- Attachment #1: Type: text/plain, Size: 3266 bytes --]

() Dan Nicolaescu <dann@ics.uci.edu>
() Wed, 20 Feb 2008 10:50:41 -0800

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

It's just a shortcut for M-x vc-status RET <ROOT> RET.  In the following
updated patch, all subdirs from root downward are buttonized (not just root).

     > ! ;;   for the files in DIR.  This function is called twice,
                                                   ^^^^^^^^^^^^^^^^
   Can you please explain why?

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

   What's the meaning of KICKP ? 

There is now a lengthy comment in vc-status-refresh that addresses
these questions.

     > ! ;;   If the backend workings are asynchronous,

   Why bother making the backend synchronous?

To support asynchronous behavior well, we wish to keep the user informed,
i.e, updating some visible status at the asynchronous boundaries (twice).
If the backend is very fast (completes below some threshold, say 0.5 sec),
this double update appears as a flickering, and is not only uninformative,
it may be downright bewildering.  Thus, a friendly backend may choose to
operate synchronously if it is confident that it can do its job under some
other reasonable threshold for user patience (say 3-5 seconds).  This is the
backend's business; vc.el should not presume to know.

You can see this consideration in play in vc-git-dir-status, which eschews
asynchronous operation completely, so confident it is.  On the other hand,
in vc-svn-dir-status, i have placed a TODO comment where someone who knows
subversion better can add code to dynamically determine how to DTRT there.

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

Yes, but removing the need to specify UPDATE-FUNCTION is better.

   - (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? 

For several reasons (w/ current patch):
 - For some backends, "working dir" and "repository" are one and the same.
 - Which backend is reflected in the mode line (re-using var vc-mode).
 - The vc-BACKEND-dir-status return value now allows the backend to
   include arbitrary backend- and/or dir-specific metainfo.
   See vc-svn-dir-status for an example.

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

Good point.  I have modified vc-status-refresh to constrain itself to the
first line; second line and onward completely up to the backend.

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

"It takes a tough man to get a tender chicken" (or something along those
lines ;--).  Please see comment in vc-status-refresh.

thi


______________________________________________________________
changelog entries same as before

full munging

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: full munging --]
[-- Type: text/x-diff, Size: 24230 bytes --]

--- vc.el.~1.538~	2008-02-21 10:48:20.000000000 +0100
+++ vc.el	2008-02-21 15:55:55.000000000 +0100
@@ -167,18 +167,24 @@
 ;;   in older versions this method was not required to recurse into
 ;;   subdirectories.)
 ;;
-;; - dir-status (dir update-function status-buffer)
+;; - dir-status (kickp)
 ;;
-;;   Produce RESULT: a list of conses of the form (file . vc-state)
-;;   for the files in DIR.  If a command needs to be run to compute
-;;   this list, it should be run asynchronously.  When RESULT is
-;;   computed, it should be passed back by doing:
-;;       (funcall UPDATE-FUNCTION RESULT STATUS-BUFFER)
 ;;   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.
+;;   change.  This is a replacement for dir-state.
+;;
+;;   Produce a list (BLURB RESULT).  BLURB is a possibly-multiline,
+;;   newline-terminated string (or the empty string "").  RESULT is a
+;;   list of conses of the form (file . vc-state) for the files in
+;;   DIR.  This function is called twice, the first time with KICKP t,
+;;   the second time, with KICKP nil.  In both calls, the current
+;;   buffer is a scratch buffer with `default-directory' set
+;;   appropriately.  If the backend workings are asynchronous, it must
+;;   start the subprocess when KICKP is t, using the current buffer as
+;;   its process buffer.  The return value of the second call is the
+;;   above-described list.  See comments in `vc-status-refresh' for
+;;   more info.
 ;;
 ;; * working-revision (file)
 ;;
@@ -606,6 +612,7 @@
 (eval-when-compile
   (require 'cl)
   (require 'compile)
+  (require 'button)
   (require 'dired)      ; for dired-map-over-marks macro
   (require 'dired-aux))	; for dired-kill-{line,tree}
 
@@ -918,6 +925,10 @@
 (defvar vc-dired-mode nil)
 (make-variable-buffer-local 'vc-dired-mode)
 
+(defsubst vc-overview-p ()
+  "Return non-nil if current buffer is in VC Dired or VC Status mode."
+  (memq major-mode '(vc-dired-mode vc-status-mode)))
+
 ;; File property caching
 
 (defun vc-clear-context ()
@@ -1794,9 +1805,7 @@
 \(current one if no file).  AFTER-HOOK specifies the local value
 for `vc-log-after-operation-hook'."
   (let ((parent
-         (if (eq major-mode 'vc-dired-mode)
-             ;; If we are called from VC dired, the parent buffer is
-             ;; the current buffer.
+         (if (vc-overview-p)
              (current-buffer)
            (if (and files (equal (length files) 1))
                (get-file-buffer (car files))
@@ -1934,7 +1943,7 @@
   ;; Sync parent buffer in case the user modified it while editing the comment.
   ;; But not if it is a vc-dired buffer.
   (with-current-buffer vc-parent-buffer
-    (or vc-dired-mode (vc-buffer-sync)))
+    (unless (vc-overview-p) (vc-buffer-sync)))
   (if (not vc-log-operation)
       (error "No log operation is pending"))
   ;; save the parameters held in buffer-local variables
@@ -2641,12 +2650,7 @@
   name)
 
 (defvar vc-status nil)
-
-(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)))
+(defvar vc-status-overlay nil)
 
 (defun vc-status-printer (fileentry)
   "Pretty print FILEENTRY."
@@ -2673,12 +2677,27 @@
 
 ;;;###autoload
 (defun vc-status (dir)
-  "Show the VC status for 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: ")
-  (vc-setup-buffer "*vc-status*")
-  (switch-to-buffer "*vc-status*")
-  (cd dir)
-  (vc-status-mode))
+  (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,42 +2796,177 @@
 
 (defun vc-status-mode ()
   "Major mode for VC status.
+Prepare the buffer to begin with the line:
+
+Directory: DEFAULT-DIRECTORY
+
+In DEFAULT-DIRECTORY, all filename components starting from the
+project's \"root\" directory are displayed as buttons whose action
+is to run command `vc-status' in the respective directory.
+
+Keys do not self-insert; instead they do different things:
 \\{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)
+  (buffer-disable-undo)
+  (erase-buffer)
+  (let* ((backend (vc-responsible-backend default-directory))
+         (find-root (vc-find-backend-function backend 'root))
+         ;; Use `(or (when ...))' in case `find-root' => nil.
+         (root (or (when find-root
+                     (funcall find-root default-directory))
+                   default-directory)))
+    (setq major-mode 'vc-status-mode
+          mode-name "VC Status"
+          vc-mode (symbol-name backend))
+    (insert "Directory: ")
+    (let* ((parent (directory-file-name
+                    (file-name-directory
+                     (directory-file-name root))))
+           (p (point))
+           (components (split-string (substring default-directory
+                                                (length parent))
+                                     "/" t))
+           full)
+      (insert parent)
+      ;; Make buttons for each directory from root down.  Mice, feh.
+      ;; (For some backends, this degenerates to simply default dir.)
+      (dolist (name components)
+        (insert "/")
+        (setq full (concat (buffer-substring-no-properties p (point)) name))
+        (insert-text-button
+         name
+         'dir full
+         'action (lambda (button)
+                   (vc-status (button-get button 'dir)))
+         'follow-link t)))
+    ;; Add some whitespace and then a placeholder character that hosts
+    ;; the overlay for displaying refresh progress (timestamp, "working").
+    (insert "     -")
+    (set (make-local-variable 'vc-status-overlay)
+         (make-overlay (1- (point)) (point)))
+    (insert "\n")
     (set (make-local-variable 'vc-status)
-	 (ewoc-create #'vc-status-printer
-		      (vc-status-headers backend default-directory)))
-    (vc-status-refresh)))
+         (ewoc-create #'vc-status-printer))
+    (use-local-map vc-status-mode-map)
+    (setq buffer-read-only t)))
 
 (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."
+  "Refresh the contents of the VC Status buffer.
+Display at end of first line the HH:MM:SS when the buffer was refreshed.
+Display backend-specific info starting from the second line.
+Lastly, display `fileinfo' entries, one per line.
+
+If the backend works asynchronously, display \"(BACKEND working)\"
+following the timestamp, and arrange for subsequent calls to
+`vc-status-refresh' (while still working) to signal error."
   (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))))
+  (unless vc-status
+    (error "Not in a VC Status buffer"))
+  (when mode-line-process
+    (error "Refresh in progress (please wait, or kill buffer)"))
+  (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))
+         ;; We manage the scratch buffer, instead of letting the backend
+         ;; handle it, for two reasons: (a) it's easy to extract process
+         ;; status from that buffer since we know about it; (b) reducing
+         ;; potential programming error in the backend is Good Planning.
+         (scratch (get-buffer-create (format " vc status: %s"
+                                             default-directory)))
+         notice)
+    ;; We used to do this: Call the backend function, passing it the
+    ;; default directory, a callback, and the buffer `here'; require
+    ;; that the backend call the callback with its result and (again)
+    ;; `here'; require that the callback do its thing in buffer `here';
+    ;; implement a callback that satisfied the requirement.
+    ;;
+    ;; This was very general, but proved suboptimal in practice:
+    ;; - There was only one function ever passed as the callback,
+    ;;   so that variability just introduced failure modes.
+    ;; - Likewise for the (lone) callback, there is only one family
+    ;;   of callers; to handle inappropriate calls would require
+    ;;   more arg checking, and "intention synchronization".
+    ;; - Each backend managed its own temporary process buffer,
+    ;;   sometimes buggily (eg, never discarding old buffers),
+    ;;   and there was no way to get process status info.
+    ;; - The default directory of buffer `here' can be computed from
+    ;;   `here', so that variability just introduced failure modes.
+    ;;
+    ;; These problems can all be lumped under the concept "unneeded
+    ;; exposure": More functions, more arguments, more "should call"
+    ;; sequences, more ways to shoot yourself in the foot.  To remedy
+    ;; this, we move (most of) the inherent (irreducible) complexity of
+    ;; an asynchronous-support architecture from the (call)stack to the
+    ;; buffer, a central and strongly-supported data structure in Emacs.
+    ;;
+    ;; Essentially the transformation is: "Stay put!"  That is, rather
+    ;; than passing location information around, establish fixed locations
+    ;; and arrange for the backend to be called "there".  More concretely,
+    ;; this means we manage the buffers -- including lifetime of scratch
+    ;; buffer -- in one function, using `with-current-buffer' and other
+    ;; available "current buffer" support, for specifying input (output is
+    ;; still returned on the stack).
+    ;;
+    ;; In exchange for this simplicity, the backend must conform to a
+    ;; "two-phase" calling sequence.  In the first phase, "kick", the
+    ;; backend starts the subprocess; in the second phase, "collect",
+    ;; it does the rest of the work to compute its result.  Thus, the
+    ;; only argument the backend needs is PHASE.
+    ;;
+    ;;  (defun vc-BACKEND-dir-status (phase)
+    ;;    (case phase (kick (START-ASYNC-SUBPROCESS))
+    ;;                (collect (let (result)
+    ;;                           (GROVEL-OVER-OUTPUT)
+    ;;                           result))))
+    ;;
+    ;; Furthermore, there are only two phases, so this can be
+    ;; represented by a boolean, KICKP.
+    ;;
+    ;;  (defun vc-BACKEND-dir-status (kickp)
+    ;;    (if kickp
+    ;;        (START-ASYNC-SUBPROCESS)
+    ;;      (let (result)
+    ;;        (GROVEL-OVER-OUTPUT)
+    ;;        result)))
+    ;;
+    ;; Note the backend is not required to work asynchronously.
+    ;; (This has not changed from before, comments notwithstanding. ;-)
+    ;;
+    ;; 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))
+      (overlay-put vc-status-overlay 'display
+                   (format "%s (%s working)" (format-time-string "%T")
+                           backend))
+      (setq mode-line-process notice))
+    (with-current-buffer scratch
+      (vc-exec-after
+       ;; ... then collect.
+       `(let* ((tuple (,get-status nil))
+               (blurb (pop tuple))
+               (entries (pop tuple)))
+          (when (buffer-live-p ,here)
+            (with-current-buffer ,here
+              (ewoc-filter vc-status 'ignore)
+              (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 blurb (if first "" "(no entries)"))
+                (overlay-put vc-status-overlay 'display
+                             (format-time-string "%T"))
+                (setq mode-line-process nil))))
+          (kill-buffer nil))))))
 
 (defun vc-status-next-line (arg)
   "Go to the next line.
--- vc-svn.el.~1.70~	2008-02-21 15:56:53.000000000 +0100
+++ vc-svn.el	2008-02-21 14:51:29.000000000 +0100
@@ -158,34 +158,32 @@
       (vc-svn-command t 0 nil "status" (if localp "-v" "-u"))
       (vc-svn-parse-status))))
 
-(defun vc-svn-after-dir-status (callback buffer)
-  (let ((state-map '((?A . added)
-                    (?C . edited)
-                    (?D . removed)
-                    (?I . ignored)
-                    (?M . edited)
-                    (?R . removed)
-                    (?? . unregistered)
-                    ;; This is what vc-svn-parse-status does.
-                    (?~ . edited)))
-       result)
-    (goto-char (point-min))
-    (while (re-search-forward "^\\(.\\)..... \\(.*\\)$" nil t)
-      (let ((state (cdr (assq (aref (match-string 1) 0) state-map)))
-           (filename (match-string 2)))
-       (when state
-         (setq result (cons (cons filename state) result)))))
-    (funcall callback result buffer)))
-
-(defun vc-svn-dir-status (dir callback buffer)
-  "Run 'svn status' for DIR and update BUFFER via CALLBACK.
-CALLBACK is called as (CALLBACK RESULT BUFFER), where
-RESULT is a list of conses (FILE . STATE) for directory DIR."
-  (with-current-buffer (get-buffer-create
-                       (generate-new-buffer-name " *vc svn status*"))
-    (vc-svn-command (current-buffer) 'async nil "status")
-    (vc-exec-after
-     `(vc-svn-after-dir-status (quote ,callback) ,buffer))))
+(defun vc-svn-dir-status (kickp)
+  "Return a list of conses (FILE . STATE) for the default directory."
+  (if kickp
+      ;; TODO: Conditionally synchronous.
+      (vc-svn-command (current-buffer) 'async nil "status")
+    (let ((state-map '((?A . added)
+                       (?C . edited)
+                       (?D . removed)
+                       (?I . ignored)
+                       (?M . edited)
+                       (?R . removed)
+                       (?? . unregistered)
+                       ;; This is what vc-svn-parse-status does.
+                       (?~ . edited)))
+          result)
+      (goto-char (point-min))
+      (while (re-search-forward "^\\(.\\)..... \\(.*\\)$" nil t)
+        (let ((state (cdr (assq (aref (match-string 1) 0) state-map)))
+              (filename (match-string 2)))
+          (when state
+            (setq result (cons (cons filename state) result)))))
+      (list (shell-command-to-string
+             ;; TODO: Make customizable.
+             ;;"svn info . | sed '/Revision:/!d'"
+             "svn info . | sed '/Path:/d;/Node Kind:/d'")
+            result))))
 
 (defun vc-svn-working-revision (file)
   "SVN-specific version of `vc-working-revision'."
--- vc-hg.el.~1.50~	2008-02-21 10:30:55.000000000 +0100
+++ vc-hg.el	2008-02-21 11:19:01.000000000 +0100
@@ -483,42 +483,34 @@
 
 (define-derived-mode vc-hg-incoming-mode vc-hg-log-view-mode "Hg-Incoming")
 
-;; XXX Experimental function for the vc-dired replacement.
-(defun vc-hg-after-dir-status (update-function buff)
-  (let ((status-char nil)
-	(file nil)
-	(translation '((?= . up-to-date)
-		       (?C . up-to-date)
-		       (?A . added)
-		       (?R . removed)
-		       (?M . edited)
-		       (?I . ignored)
-		       (?! . deleted)
-		       (?? . unregistered)))
-	(translated nil)
-	  (result nil))
+(defun vc-hg-dir-status (kickp)
+  "Return a list of conses (FILE . STATE) for the default directory."
+  (if kickp
+      ;; TODO: Conditionally synchronous.
+      (vc-hg-command (current-buffer) 'async default-directory "status")
+    (let ((status-char nil)
+          (file nil)
+          (translation '((?= . up-to-date)
+                         (?C . up-to-date)
+                         (?A . added)
+                         (?R . removed)
+                         (?M . edited)
+                         (?I . ignored)
+                         (?! . deleted)
+                         (?? . unregistered)))
+          (translated nil)
+          (result nil))
       (goto-char (point-min))
       (while (not (eobp))
-	(setq status-char (char-after))
-	(setq file
-	      (buffer-substring-no-properties (+ (point) 2)
-					      (line-end-position)))
-	(setq translated (assoc status-char translation))
-	(when (and translated (not (eq (cdr translated) 'up-to-date)))
-	  (push (cons file (cdr translated)) result))
-	(forward-line))
-      (funcall update-function result buff)))
-
-;; XXX Experimental function for the vc-dired replacement.
-(defun vc-hg-dir-status (dir update-function status-buffer)
-  "Return a list of conses (file . state) for DIR."
-  (with-current-buffer
-      (get-buffer-create
-       (expand-file-name " *VC-hg* tmp status" dir))
-    (erase-buffer)
-    (vc-hg-command (current-buffer) 'async dir "status")
-    (vc-exec-after
-     `(vc-hg-after-dir-status (quote ,update-function) ,status-buffer))))
+        (setq status-char (char-after))
+        (setq file
+              (buffer-substring-no-properties (+ (point) 2)
+                                              (line-end-position)))
+        (setq translated (assoc status-char translation))
+        (when (and translated (not (eq (cdr translated) 'up-to-date)))
+          (push (cons file (cdr translated)) result))
+        (forward-line))
+      (list "" result))))
 
 ;; XXX this adds another top level menu, instead figure out how to
 ;; replace the Log-View menu.
--- vc-git.el.~1.38~	2008-02-21 10:31:02.000000000 +0100
+++ vc-git.el	2008-02-21 15:46:21.000000000 +0100
@@ -207,52 +207,53 @@
       ;; fall back to the default VC representation
       (vc-default-dired-state-info 'Git file))))
 
-;;; vc-dir-status support (EXPERIMENTAL)
-;;; If vc-directory (which is not half bad under Git, w/ some tweaking)
-;;; is to go away, vc-dir-status must at least support the same operations.
-;;; At the moment, vc-dir-status design is still fluid (a kind way to say
-;;; half-baked, undocumented, and spottily-supported), so the following
-;;; should be considered likewise ripe for sudden unannounced change.
-;;; YHBW, HAND.  --ttn
-
-(defun vc-git-after-dir-status (callback buffer)
-  (sort-regexp-fields t "^. \\(.+\\)$" "\\1" (point-min) (point-max))
-  (let ((map '((?H . cached)
-               (?M . unmerged)
-               (?R . removed)
-               (?C . edited)
-               (?K . removed)           ; ??? "to be killed"
-               (?? . unregistered)))
-        status filename result)
-    (goto-char (point-min))
-    (while (> (point-max) (point))
-      (setq status (string-to-char (buffer-substring (point) (1+ (point))))
-            status (cdr (assq status map))
-            filename (buffer-substring (+ 2 (point)) (line-end-position)))
-      ;; TODO: Add dynamic selection of which status(es) to display, and
-      ;; bubble that up to vc-dir-status.  For now, we consider `cached'
-      ;; to be uninteresting, to mimic vc-directory (somewhat).
-      (unless (eq 'cached status)
+(defun vc-git-dir-status (kickp)
+  "Return a list of conses (FILE . STATE) for the default directory."
+  ;; Don't do it asynchronously; git is fast and always local.
+  ;; (vc-git-command (current-buffer) 'async default-directory "status")
+  (unless kickp
+    ;; Avoid "-a" so as to be able to distinguish "in index".
+    (call-process "git" nil t nil "status")
+    (let* ((root (vc-git-root default-directory))
+           (sub (file-relative-name default-directory root))
+           ;; If we are not in the project's root dir, discard
+           ;; lines that do not have the relative-dir prefix.
+           (keep-rx (concat "^#\t\\([^:]+\\): +"
+                            (if (member sub '("." "./"))
+                                ""
+                              (file-name-as-directory sub))))
+           (pair-rx (concat keep-rx "\\(.+\\)$"))
+           status filename result)
+      (goto-char (point-min))
+      ;; Encode "in index" in the state; eg: `modified' vs `modified/in'.
+      (when (search-forward "\n# Changes to be committed:\n" nil t)
+        (search-forward "#\t")
+        (forward-char -2)
+        (while (looking-at "#\t[^:]+\\(:\\)")
+          (replace-match "/in:" t t nil 1)
+          (forward-line 1)))
+      (when (search-forward "\n# Untracked files:\n" nil t)
+        (while (re-search-forward "^#\t" nil t)
+          (insert "untracked: ")))
+      (keep-lines keep-rx (point-min) (point-max))
+      ;; This sorting is purely cosmetic.  We will probably remove it a
+      ;; little further down the road, when VC Status learns to manage
+      ;; total ordering and all that jazz.  --ttn
+      (sort-regexp-fields t pair-rx "\\2" (point-min) (point-max))
+      (goto-char (point-min))
+      (while (re-search-forward pair-rx nil t)
+        (setq status (match-string 1)
+              status (if (string-match "new file" status)
+                         (replace-match "new" t t status)
+                       status)
+              status (intern status)
+              filename (match-string 2))
+        (when (memq status '(renamed renamed/in copied copied/in))
+          ;; Discard first name: "ONE -> TWO" becomes "TWO".
+          (setq filename (substring filename
+                                    (+ 4 (string-match " -> " filename)))))
         (push (cons filename status) result))
-      (forward-line 1))
-    (funcall callback result buffer)))
-
-(defun vc-git-dir-status (dir update-function status-buffer)
-  "Return a list of conses (file . state) for DIR."
-  (with-current-buffer
-      (get-buffer-create
-       (expand-file-name " *VC-Git* tmp status" dir))
-    (erase-buffer)
-    (vc-git-command (current-buffer) 'async dir "ls-files" "-t"
-                    "-c"                ; cached
-                    "-d"                ; deleted
-                    "-k"                ; killed
-                    "-m"                ; modified
-                    "-o"                ; others
-                    "--directory"
-                    "--exclude-per-directory=.gitignore")
-    (vc-exec-after
-     `(vc-git-after-dir-status (quote ,update-function) ,status-buffer))))
+      (list "" result))))
 
 ;;; STATE-CHANGING FUNCTIONS
 

Re: vc-*-root finctions

[Dan Nicolaescu]

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

  > () Dan Nicolaescu <dann@ics.uci.edu>
  > () Wed, 20 Feb 2008 10:50:41 -0800
  > 
  >    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...
  > 
  > It's just a shortcut for M-x vc-status RET <ROOT> RET.  In the following
  > updated patch, all subdirs from root downward are buttonized (not just root).

It seems that this patch contains multiple independent things.  It would
be better to split them, so they can be judged separately.


  >      > ! ;;   for the files in DIR.  This function is called twice,
  >                                                    ^^^^^^^^^^^^^^^^
  >    Can you please explain why?
  > 
  >      > ! ;;   time with KICKP t, the second time, with KICKP nil.
  > 
  >    What's the meaning of KICKP ? 
  > 
  > There is now a lengthy comment in vc-status-refresh that addresses
  > these questions.

As a documentation stickler you should know that having a backend
implementer look for documentation in both the beginning of vc.el and in
the comment in vc-status-refresh is not a good idea.  More, the comment
in vc-status-refresh talks about "We used to do this:" is just
pointless.

Frankly, having a function that is called twice once with an argument
t and another time with nil, and does completely different things
depending on the argument just screams WEIRD.  Better have 2 different
functions.

The fact that there's a need for about 70 lines of comments for one
backend function, points to some design problems.

  >      > ! ;;   If the backend workings are asynchronous,
  > 
  >    Why bother making the backend synchronous?
  > 
  > To support asynchronous behavior well, we wish to keep the user informed,
  > i.e, updating some visible status at the asynchronous boundaries (twice).

Huh? Please explain this.

In general, it hard to see what you are trying to accomplish here, and
why you changes are better than what is there now.

  > If the backend is very fast (completes below some threshold, say 0.5 sec),
  > this double update appears as a flickering, and is not only uninformative,
  > it may be downright bewildering.  Thus, a friendly backend may choose to
  > operate synchronously if it is confident that it can do its job under some
  > other reasonable threshold for user patience (say 3-5 seconds).  This is the
  > backend's business; vc.el should not presume to know.

You haven't explained why it is better to introduce the synchronous
behavior when I said that it can be easily used with the current API.

  > You can see this consideration in play in vc-git-dir-status, which eschews
  > asynchronous operation completely, so confident it is.

Which would be wrong.  git might be fast, but it takes a long time if it
has to read the inodes from disk or NFS on a big tree (which happens
every morning or after a big compilation job).

  > in vc-svn-dir-status, i have placed a TODO comment where someone who knows
  > subversion better can add code to dynamically determine how to DTRT there.
  > 
  >    The current API can be used by a synchronous backend too.  It just needs
  >    to call the UPDATE-FUNCTION when done processing.
  > 
  > Yes, but removing the need to specify UPDATE-FUNCTION is better.

Why is that a big problem.  The usage model is very simple, and it could
be easily extended to deal with doing incremental update, which we
haven't yet completely excluded as unnecessary.  Avoiding
UPDATE-FUNCTION can be done by passing instead a buffer in which
dir-status could do its thing (and I think Tom Tromey had a patch for
that). But it's hard to see that making any difference, there are many
other things to worry about now...

  >    - (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? 
  > 
  > For several reasons (w/ current patch):
  >  - For some backends, "working dir" and "repository" are one and the same.
  >  - Which backend is reflected in the mode line (re-using var vc-mode).
  >  - The vc-BACKEND-dir-status return value now allows the backend to
  >    include arbitrary backend- and/or dir-specific metainfo.
  >    See vc-svn-dir-status for an example.

Yeah, this was intended to call a backend specific function at some
point, so better keep it a separate function that mix this code in the
middle of something else.

+(defsubst vc-overview-p ()
+  "Return non-nil if current buffer is in VC Dired or VC Status mode."
+  (memq major-mode '(vc-dired-mode vc-status-mode)))

There are a few other places that could use this.  But this should go at
the time vc-dired is completely replaced.  Why not check this in
separately?

Re: vc-*-root finctions

[Tom Tromey]

>>>>> "Dan" == Dan Nicolaescu <dann@ics.uci.edu> writes:

>> To support asynchronous behavior well, we wish to keep the user informed,
>> i.e, updating some visible status at the asynchronous boundaries (twice).

Dan> Huh? Please explain this.

He is saying that when you start an async command, you want to give
the user some indication is has started; and then when the command
finishes, Emacs should give an indication that it has finished.

That seems fairly uncontroversial to me.

However, I think the most likely cause of "flickering" would be the
pcvs-like in-buffer notification, which got nixed.  Messages in the
echo area seem much less likely to be problematic.

Tom

Re: vc-*-root finctions

[Dan Nicolaescu]

Tom Tromey <tromey@redhat.com> writes:

  > >>>>> "Dan" == Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  > >> To support asynchronous behavior well, we wish to keep the user informed,
  > >> i.e, updating some visible status at the asynchronous boundaries (twice).
  > 
  > Dan> Huh? Please explain this.
  > 
  > He is saying that when you start an async command, you want to give
  > the user some indication is has started; and then when the command
  > finishes, Emacs should give an indication that it has finished.
  > 
  > That seems fairly uncontroversial to me.

Agreed.  That's why I said that the patch needs to be split up.... 
Using the mode-line (like all the vc-commands do now) seems like a good
idea.

  > However, I think the most likely cause of "flickering" would be the
  > pcvs-like in-buffer notification, which got nixed.  

Printing the exact command run for doing was considered unnecessary, but
maybe printing some other status message could be helpful...

Re: vc-*-root finctions

[Stefan Monnier]

> Which would be wrong.  git might be fast, but it takes a long time if it
> has to read the inodes from disk or NFS on a big tree (which happens
> every morning or after a big compilation job).

BTW, now that I think a bit more about it: the `dir-status' function can
almost always be synchronous (and never contact a remote repository).
In PCL-CVS that function is called `cvs-quickdir', which I consider to
be the main entry point.

[ Oddly Arch is one of the very few VCS for which figuring out the local
  status of a file may *require* contacting a remote repository, so it's
  probably the only backend for which an async dir-status would
  be necessary. ]

The asynchronous behavior is only important for all the other operations
you'll want to perform later (once you already have a vc-status buffer
with which to work): update/merge/pull, commit/push.


        Stefan

Re: vc-*-root finctions

[Tom Tromey]

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

Stefan> The asynchronous behavior is only important for all the other
Stefan> operations you'll want to perform later (once you already have
Stefan> a vc-status buffer with which to work): update/merge/pull,
Stefan> commit/push.

On my gcc tree, a plain "svn status" takes a full minute (at the top
level).  I think that is much too long a time for Emacs to be
unresponsive.  So, I would prefer async operation for the dir-status
by default -- right now I simply can't use vc-dired for my work, and I
would like to be able to use something.

I do agree that a particular back end could choose to work
synchronously.

Tom

Re: vc-*-root finctions

[Dan Nicolaescu]

Tom Tromey <tromey@redhat.com> writes:

  > >>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:
  > 
  > Stefan> The asynchronous behavior is only important for all the other
  > Stefan> operations you'll want to perform later (once you already have
  > Stefan> a vc-status buffer with which to work): update/merge/pull,
  > Stefan> commit/push.
  > 
  > On my gcc tree, a plain "svn status" takes a full minute (at the top
  > level).  I think that is much too long a time for Emacs to be
  > unresponsive.  So, I would prefer async operation for the dir-status
  > by default -- right now I simply can't use vc-dired for my work, and I
  > would like to be able to use something.
  > 
  > I do agree that a particular back end could choose to work
  > synchronously.

The current implementation does not care if the backend is asynchronous
or not, the backend just needs to call the UPDATE-FUNCTION when it's
done computing the status (I'm sure you knew that, this is just for
everybody else's benefit)...

Re: vc-*-root finctions

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > > Which would be wrong.  git might be fast, but it takes a long time if it
  > > has to read the inodes from disk or NFS on a big tree (which happens
  > > every morning or after a big compilation job).
  > 
  > BTW, now that I think a bit more about it: the `dir-status' function can
  > almost always be synchronous (and never contact a remote repository).
  > In PCL-CVS that function is called `cvs-quickdir', which I consider to
  > be the main entry point.

I am not sure I understand what you are saying... 
Never contacting the remote repository does not mean that it won't block
for a long time, on big trees it happens even with the current crop of
fast VCSes.

Re: vc-*-root finctions

[Thien-Thi Nguyen]

() Dan Nicolaescu <dann@ics.uci.edu>
() Thu, 21 Feb 2008 10:35:38 -0800

   It seems that this patch contains multiple independent things.  It would
   be better to split them, so they can be judged separately.

Good point.  Will do so in the next few days.

   As a documentation stickler you should know that having a backend
   implementer look for documentation in both the beginning of vc.el and in
   the comment in vc-status-refresh is not a good idea.

Why is not a good idea?  One explains the interface, the other the rationale.

   More, the comment in vc-status-refresh talks about "We used to
   do this:" is just pointless.

Lots of comments in Emacs are likewise pointless, i suppose.  In
any case, i'll move it to top-level and make it less time-oriented.

   Frankly, having a function that is called twice once with an argument
   t and another time with nil, and does completely different things
   depending on the argument just screams WEIRD.  Better have 2 different
   functions.

I disagree.  More functions is more maintenance.

   The fact that there's a need for about 70 lines of comments for one
   backend function, points to some design problems.

How so?

   In general, it hard to see what you are trying to accomplish here, and
   why you changes are better than what is there now.

The changes try to reduce failure modes, which, being "potential"
rather than actual issues, may indeed be difficult to appreciate.

   You haven't explained why it is better to introduce the synchronous
   behavior when I said that it can be easily used with the current API.

Well, i haven't introduced synchronous behavior in vc.el, so why
would i want to explain that?

   Which would be wrong.  git might be fast, but it takes a long time if it
   has to read the inodes from disk or NFS on a big tree (which happens
   every morning or after a big compilation job).

I don't know about that.  I observe that "git status" on emacs.git
takes a little longer (maybe five seconds) the first time it is
run and then less than a second for subsequent times.  (Perhaps
you're not talking about "git status" that vc-git-dir-status uses?)

     > Yes, but removing the need to specify UPDATE-FUNCTION is better.

   Why is that a big problem.

More functions, more maintenance: It's not a big problem when used
correctly; it's a big problem when misunderstood and subsequently
used incorrectly.  Occam's razor and all that.

   Yeah, this was intended to call a backend specific function at some
   point, so better keep it a separate function that mix this code in the
   middle of something else.

Regardless, the best place is during vc-status-refresh.

   +(defsubst vc-overview-p () ...)

   There are a few other places that could use this.  But this should go at
   the time vc-dired is completely replaced.  Why not check this in
   separately?

Good idea.  I will commit it in the next day or two.

In the meantime, interested git users can look at:
http://www.gnuvola.org/wip/  (vc-status-hacking)

I will henceforth be munging there instead of posting patches.

thi

Re: vc-*-root finctions

[Dan Nicolaescu]

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

  > () Dan Nicolaescu <dann@ics.uci.edu>
  > () Thu, 21 Feb 2008 10:35:38 -0800
  > 
  >    It seems that this patch contains multiple independent things.  It would
  >    be better to split them, so they can be judged separately.
  > 
  > Good point.  Will do so in the next few days.
  > 
  >    As a documentation stickler you should know that having a backend
  >    implementer look for documentation in both the beginning of vc.el and in
  >    the comment in vc-status-refresh is not a good idea.
  > 
  > Why is not a good idea?  One explains the interface, the other the rationale.

Because if forces a backend implementor to read two things instead of one.

  >    Frankly, having a function that is called twice once with an argument
  >    t and another time with nil, and does completely different things
  >    depending on the argument just screams WEIRD.  Better have 2 different
  >    functions.
  > 
  > I disagree.  More functions is more maintenance.

Well, you are effectively introducing two functions, but squeezing them
into one with a very weird interface. Explain how is that allows for
less maintenance.

  >    The fact that there's a need for about 70 lines of comments for one
  >    backend function, points to some design problems.
  > 
  > How so?

How about way too complex?

  >    In general, it hard to see what you are trying to accomplish here, and
  >    why you changes are better than what is there now.
  > 
  > The changes try to reduce failure modes, which, being "potential"
  > rather than actual issues, may indeed be difficult to appreciate.

Can you please explain what you mean rather than hand wave?  Given that
you are replacing existing working code, it would be nice if you'd
explain what problem you are trying to solve, and why the approach you
took is the right way to solve the problem (other than applying the NIH principle).

  >    You haven't explained why it is better to introduce the synchronous
  >    behavior when I said that it can be easily used with the current API.
  > 
  > Well, i haven't introduced synchronous behavior in vc.el, so why
  > would i want to explain that?

You are changing code that is simple to something that is strange and
complex. Enquiring minds would like to know the reasoning behind those
changes and to make sure that the changes you are making are the right
way to solve whatever problem you are trying to solve.

  >    Which would be wrong.  git might be fast, but it takes a long time if it
  >    has to read the inodes from disk or NFS on a big tree (which happens
  >    every morning or after a big compilation job).
  > 
  > I don't know about that.  I observe that "git status" on emacs.git
  > takes a little longer (maybe five seconds) the first time it is

That's five seconds on a local disk, right?  Put that on an busy NFS
server, and do it for a bigger tree than emacs and those five seconds go
up really fast.

  >      > Yes, but removing the need to specify UPDATE-FUNCTION is better.
  > 
  >    Why is that a big problem.
  > 
  > More functions, more maintenance: It's not a big problem when used
  > correctly; it's a big problem when misunderstood and subsequently
  > used incorrectly.  Occam's razor and all that.

Occam's razor also applies to functions with terribly complicated
interfaces.  So no, the above is not a good motivation.


  >    Yeah, this was intended to call a backend specific function at some
  >    point, so better keep it a separate function that mix this code in the
  >    middle of something else.
  > 
  > Regardless, the best place is during vc-status-refresh.

And why is it better to not have a separate function?

  >    +(defsubst vc-overview-p () ...)
  > 
  >    There are a few other places that could use this.  But this should go at
  >    the time vc-dired is completely replaced.  Why not check this in
  >    separately?
  > 
  > Good idea.  I will commit it in the next day or two.
  > 
  > In the meantime, interested git users can look at:
  > http://www.gnuvola.org/wip/  (vc-status-hacking)
  > 
  > I will henceforth be munging there instead of posting patches.

As you wish. But please don't commit anything that changes the way
vc-status works before posting it here.

Re: vc-*-root finctions

[Thien-Thi Nguyen]

() Dan Nicolaescu <dann@ics.uci.edu>
() Fri, 22 Feb 2008 07:42:28 -0800

   Because if forces a backend implementor to read two things
   instead of one.

Not really.  All you need to know to implement is in the
Commentary.  The rationale doesn't add any more information,
just design background.

   Well, you are effectively introducing two functions, but
   squeezing them into one with a very weird interface.  Explain
   how is that allows for less maintenance.

Look at the overall picture.  In the present system, each
backend must decide synchronicity, do scratch buffer and
subprocess maintenance, compute the result, and do a function
call.  This can be error prone (vc-svn-dir-status from vc-svn.el
1.71 leaks buffers, e.g).  In the proposed system, the backend
need only decide synchronicity and compute its result;
vc-status-refresh does the rest.

True, vc-status-refresh becomes more complex, but the overall
reduction in (redundant) complexity is a win.

   How about way too complex?

Just stick w/ the Commentary, then; that explains the interface
requirements.  Lots of things are complex in the core so that
the interfaces are simple and regular.  This is one of them.

   Can you please explain what you mean rather than hand wave?

   Given that you are replacing existing working code, it would
   be nice if you'd explain what problem you are trying to
   solve, and why the approach you took is the right way to
   solve the problem (other than applying the NIH principle).

I have tried to explain it but since i don't know where you fail
to understand things, i don't know how to explain it better.
When i fixed the vc-svn.el 1.71 bug, that was when i started to
think about eliminating the possibility of that bug.  Perhaps if
you go through the process of fixing that bug, then think along
those lines, you will arrive at similar conclusions.

   You are changing code that is simple to something that is
   strange and complex.  Enquiring minds would like to know the
   reasoning behind those changes and to make sure that the
   changes you are making are the right way to solve whatever
   problem you are trying to solve.

You ask for an explanation.  I wrote it in a comment.  You say
the comment is too complex.  But, the important thing is: it
exists.  I can now do other things while you try to understand
it.  Good luck!

   Occam's razor also applies to functions with terribly
   complicated interfaces.  So no, the above is not a good
   motivation.

What is good and what is not good?  Are bugs good?  Are systems
that breed bugs good?  Are programmers who support systems that
breed bugs good?

   And why is it better to not have a separate function?

You repeat your questions, so i repeat my answers.  See above.

thi

Re: vc-*-root finctions

[Dan Nicolaescu]

Thien-Thi Nguyen <ttn@gnuvola.org> writes:

  > () Dan Nicolaescu <dann@ics.uci.edu>
  > () Fri, 22 Feb 2008 07:42:28 -0800
  > 
  >    Because if forces a backend implementor to read two things
  >    instead of one.
  > 
  > Not really.  All you need to know to implement is in the
  > Commentary.  The rationale doesn't add any more information,
  > just design background.

I asked what is KICKP because it's not a familiar predicate name and you
referred to the commentary in the code.

  >    Well, you are effectively introducing two functions, but
  >    squeezing them into one with a very weird interface.  Explain
  >    how is that allows for less maintenance.
  > 
  > Look at the overall picture.  In the present system, each
  > backend must decide synchronicity, do scratch buffer and
  > subprocess maintenance, compute the result, and do a function
  > call.  

You make it sound like it's a big deal, it's not.

  > This can be error prone (vc-svn-dir-status from vc-svn.el 1.71 leaks
  > buffers, e.g).

Because it's still work in progress, Tom will surely take care of that
when he gets a chance.

  > In the proposed system, the backend need only decide synchronicity
  > and compute its result; vc-status-refresh does the rest.

Looking at this again: the KICKP silliness is completely unnecessary,
you might as well have 2 functions: vc-BACKEND-start-vc-status-command and
vc-BACKEND-process-status-result.  They are 2 different things, why
obfuscate them by putting them in the same function?

The problem is that your scheme breaks if the backend needs to run more
than one process to compute the status.  Like, for example,
vc-git-dir-state does.

When doing the current design I considered putting vc-exec-after in
vc-refresh, but decided not to precisely for that reason: not wanting to
constrain the backend to run a single command.

So yes, the current system where to vc core asks the backend
"compute the status result and tell me when you are done" has some
reasoning behind it.  (And it's nothing new, that how most asynchronous
work gets implemented).

All the synchronicity stuff is completely orthogonal to this.

  > True, vc-status-refresh becomes more complex, but the overall
  > reduction in (redundant) complexity is a win.
  > 
  >    How about way too complex?
  > 
  > Just stick w/ the Commentary, then; that explains the interface
  > requirements.  Lots of things are complex in the core so that
  > the interfaces are simple and regular.  This is one of them.

I disagree that overall model is less complex in your case.

  >    Can you please explain what you mean rather than hand wave?
  > 
  >    Given that you are replacing existing working code, it would
  >    be nice if you'd explain what problem you are trying to
  >    solve, and why the approach you took is the right way to
  >    solve the problem (other than applying the NIH principle).
  > 
  > I have tried to explain it but since i don't know where you fail
  > to understand things, i don't know how to explain it better.

This is the first time you explained "why" not only "what", it would
have been a lot easier if you would have done that from the start.  Not
adding so many unrelated issues would have helped too.

I gave you the benefit of the doubt that you might have uncovered some
unseen problem, but not it's clear that you have not.  More, the
proposed design has other core problems.

  > When i fixed the vc-svn.el 1.71 bug, that was when i started to

 What bug is that? Not sure what are you referring to, 1.71 is my commit
 that helps implement a missing feature...

Re: vc-*-root finctions

[Mike Mattie]

[-- Attachment #1: Type: text/plain, Size: 4190 bytes --]

On Thu, 21 Feb 2008 16:33:01 +0100
Thien-Thi Nguyen <ttn@gnuvola.org> wrote:

> () Dan Nicolaescu <dann@ics.uci.edu>
> () Wed, 20 Feb 2008 10:50:41 -0800
> 
[snip]
 
>    Why bother making the backend synchronous?
> 
> To support asynchronous behavior well, we wish to keep the user
> informed, i.e, updating some visible status at the asynchronous
> boundaries (twice). If the backend is very fast (completes below some
> threshold, say 0.5 sec), this double update appears as a flickering,
> and is not only uninformative, it may be downright bewildering.
> Thus, a friendly backend may choose to operate synchronously if it is
> confident that it can do its job under some other reasonable
> threshold for user patience (say 3-5 seconds).  This is the backend's
> business; vc.el should not presume to know.
> 
> You can see this consideration in play in vc-git-dir-status, which
> eschews asynchronous operation completely, so confident it is.  On
> the other hand, in vc-svn-dir-status, i have placed a TODO comment
> where someone who knows subversion better can add code to dynamically
> determine how to DTRT there.

I know a bit of svn. I'll drop a couple of notes since it might take me
a while to personally work my way to the svn backend.

async:

* checkin/checkout
* command has a URL in it
* log command

sync:

* a working copy command that affects the HEAD revision of the checkout, since svn has cached
  a copy of the checkout on disk.

  These commands must *not* have a -r revision argument or svn will attempt to contact the repository
  even if there is a local cache of that revision.

  This is actually a problem with vc on my Emacs 23.0.60.2. none of the "offline" commands work, I think
  because internally vc supplies revision arguments to all commands ?

  This includes diff and revert which often puts me back in eshell.

Notes:

If you are working with fork/exec IPC to the svn client you have to be very careful not to trust the exit status
of the client.

I encountered this issue when I wrote a perl wrapper around svn to normalize the command set. I would generate a commit
command, the client would terminate with exit status == 0, and the command failed with no effect visible in the working
copy or repository.

When doing commit commands the reliable way to see if a command worked is to make sure that svn returns a revision number
on stdout. If there is no output at all the command failed.

Also might need to be an artificial delay between rapid fire commits. My perl script generated about 13 commits in a few seconds
resulting in corruption of the working copy, locked files, and a half-committed repository, despite all commands supposedly
terminating normally.

I am not sure how much of a delay is required.

I reported a bug to svn with the logs of the event, but I did not receive a response.

Speaking of locked files, the svn status needs to be scanned after commands that update the working copy or repository
meta-data, looking for internal transactions that require a svn cleanup command to clear. best place to run a cleanup
scan is before such a command.

It will be a while before my revision control project winds it's way into vc, currently I am focused on ediff and quilt.
When it does I will transfer my knowledge from my perl script which knows alot about svn, to vc, which obsoletes my
perl script.

When I stopped working on that perl script I was seriously thinking about using the library API with bindings
instead of a fork/exec IPC. The client problems would likely disappear when accessing the internal API. Emacs
probably wouldn't be able to do this the right way without the ability to load the host system's shared
objects though.

Cheers,
Mike Mattie

P.S:

VC misfeature?

The vc command update, or C-x v + always attempts to update the working copy from the repository. When the CLI
is used in conjunction with vc, you often want to update the buffer: reload the file from disk, and update the
vc modeline information, without performing a checkout. I don't know of a way to update without checkout -
it would be nice.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: vc-*-root finctions

[Stefan Monnier]

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

Why not just provide a ".." entry to move up to the directory
directory above?

After all, if you're working on a sub-sub-dir of your project, you're
just as likely to want to look at the sub-dir as wanting to look at the
actual root.

And this feature wouldn't require any new VC-function.


        Stefan

Re: vc-*-root finctions

[Thien-Thi Nguyen]

() Stefan Monnier <monnier@iro.umontreal.ca>
() Wed, 20 Feb 2008 14:20:57 -0500

   Why not just provide a ".." entry to move up to the directory
   directory above?

   After all, if you're working on a sub-sub-dir of your project, you're
   just as likely to want to look at the sub-dir as wanting to look at the
   actual root.

   And this feature wouldn't require any new VC-function.

You can link to every dir, but not every dir belongs in the
project.  Anyway, you have a good point: why omit linking to
intervening (between default and root) dirs.  Latest patch (see
other reply) no longer discriminates.

thi

Re: vc-*-root finctions

[Stefan Monnier]

>    Why not just provide a ".." entry to move up to the directory
>    directory above?

>    After all, if you're working on a sub-sub-dir of your project, you're
>    just as likely to want to look at the sub-dir as wanting to look at the
>    actual root.

>    And this feature wouldn't require any new VC-function.

> You can link to every dir, but not every dir belongs in the
> project.

Who cares?  If the user wants to go up and it does not belong to any VCS
then we can detect it without vc-*-root, and if the user wants to go up
but it belongs to another backend, I see no reason to disobey the user.

I.e. the functionality of "going up" is fine, but it does not require
the introduction of vc-*-root.


        Stefan

Re: vc-*-root finctions

[Thien-Thi Nguyen]

() Stefan Monnier <monnier@IRO.UMontreal.CA>
() Thu, 21 Feb 2008 11:16:04 -0500

   Who cares?  If the user wants to go up and it does not belong
   to any VCS then we can detect it without vc-*-root, and if the
   user wants to go up but it belongs to another backend, I see no
   reason to disobey the user.

   I.e. the functionality of "going up" is fine, but it does not
   require the introduction of vc-*-root.

Note: The feature is not "going up", for which i would agree there
is no burning need for (yet) another way to thwart the user.

Rather, the feature is to present the line:

  Directory: ~/build/GNU/git-emacs/lisp/calendar

with "git-emacs", "lisp" and "calendar" as buttons.  If i want to
do `(vc-status "~/build/GNU")', then presenting "GNU" as plain
(non-button) text tells me -- even before i try -- that according
to vc-git-root, that directory is not in the same project i am in
at the moment.  The buttons, on the other hand, tell me -- again,
even before i try -- that those directories are in-project.

I suspect you haven't tried out the patch(es).  Have you?

thi

Re: vc-*-root finctions

[Stefan Monnier]

> Rather, the feature is to present the line:

>   Directory: ~/build/GNU/git-emacs/lisp/calendar

> with "git-emacs", "lisp" and "calendar" as buttons.  If i want to
> do `(vc-status "~/build/GNU")', then presenting "GNU" as plain
> (non-button) text tells me -- even before i try -- that according
> to vc-git-root, that directory is not in the same project i am in
> at the moment.  The buttons, on the other hand, tell me -- again,
> even before i try -- that those directories are in-project.

I understand that.  But why should VC care if it's in the same project
or not?  The user will know that just fine.  So just highlight the
button if VC can do something with it, and don't highlight it if
it can't.
Look 'ma: no need for the vc-FOO-root functions.
 

        Stefan