Re: [PATCH] Fix for vc-bzr-diff when vc-bzr-diff-switches is t

[Stefan Monnier]

> +2011-01-26  Deniz Dogan  <address@hidden>
> +
> +       * vc/vc-bzr.el (vc-bzr-diff): Don't pass --diff-options if
> +       vc-bzr-diff-switches is t.

The comment's fine.

> -  ;; `bzr diff' exits with code 1 if diff is non-empty.
> -  (apply #'vc-bzr-command "diff" (or buffer "*vc-diff*")
> -        (if vc-disable-async-diff 1 'async) files
> -         "--diff-options" (mapconcat 'identity
> -                                     (vc-switches 'bzr 'diff)
> -                                    " ")
> -         ;; This `when' is just an optimization because bzr-1.2 is *much*
> -         ;; faster when the revision argument is not given.
> -         (when (or rev1 rev2)
> -           (list "-r" (format "%s..%s"
> -                              (or rev1 "revno:-1")
> -                              (or rev2 ""))))))
> +  (let (args)
> +    (unless (eq t vc-bzr-diff-switches) ;; t means "no switches"
> +      (setq args
> +            (list "--diff-options" (mapconcat 'identity
> +                                              (vc-switches 'bzr 'diff)
> +                                              " "))))
> +    ;; This `when' is just an optimization because bzr-1.2 is *much*
> +    ;; faster when the revision argument is not given.
> +    (when (or rev1 rev2)
> +      (setq args (append args (list "-r" (format "%s..%s"
> +                                                 (or rev1 "revno:-1")
> +                                                 (or rev2 ""))))))
> +    ;; `bzr diff' exits with code 1 if diff is non-empty.
> +    (apply #'vc-bzr-command "diff" (or buffer "*vc-diff*")
> +           (if vc-disable-async-diff 1 'async) files
> +           args)))

I suggest you try hard to directly give a value within `let' rather than
use (let (var) ...) and then give a value to var via `setq'.
In the above case, it would look like:

     (let ((args
            (append
             (unless (eq t vc-bzr-diff-switches) ;; t means "no switches"
               (list "--diff-options" (mapconcat 'identity
                                                 (vc-switches 'bzr 'diff)
                                                 " ")))
             ;; This `when' is just an optimization because bzr-1.2 is *much*
             ;; faster when the revision argument is not given.
             (when (or rev1 rev2)
               (list "-r" (format "%s..%s"
                                  (or rev1 "revno:-1")
                                  (or rev2 "")))))))
       ;; `bzr diff' exits with code 1 if diff is non-empty.
       (apply #'vc-bzr-command "diff" (or buffer "*vc-diff*")
              (if vc-disable-async-diff 1 'async) files
              args)))

which you can also turn into

     ;; `bzr diff' exits with code 1 if diff is non-empty.
     (apply #'vc-bzr-command "diff" (or buffer "*vc-diff*")
            (if vc-disable-async-diff 1 'async) files
            (append
             (unless (eq t vc-bzr-diff-switches) ;; t means "no switches"
               (list "--diff-options" (mapconcat 'identity
                                                 (vc-switches 'bzr 'diff)
                                                 " ")))
             ;; This `when' is just an optimization because bzr-1.2 is *much*
             ;; faster when the revision argument is not given.
             (when (or rev1 rev2)
               (list "-r" (format "%s..%s"
                                  (or rev1 "revno:-1")
                                  (or rev2 ""))))))

if you prefer.  Also I agree with Glenn that it's better to check for
(zerop (length (vc-switches 'bzr 'diff))).


        Stefan