Xavier Maillard <zedek@xxxxxxx> writes:
> I thought it would be cooler to have different set of colors each time
> I blame.
But the code for it looks weird:
> @@ -302,9 +320,8 @@ See also function `git-blame-mode'."
> (inhibit-point-motion-hooks t)
> (inhibit-modification-hooks t))
> (when (not info)
> - (let ((color (pop git-blame-colors)))
> - (unless color
> - (setq color git-blame-ancient-color))
> + (let ((color (or (elt git-blame-colors (random (length git-blame-colors)))
> + git-blame-ancient-color)))
> (setq info (list hash src-line res-line num-lines
> (git-describe-commit hash)
> (cons 'color color))))
Instead of using the colors one at a time, you randomly select one of
them. This means that you might select the same color twice or more,
and even twice in a row. And you will never run out of colors, so
git-blame-ancient-color will never be used.
This change should probably not go in, but your patch has other stuff
that's good.
> * Prevent (future possible) namespace clash by renaming `color-scale'
> into `git-blame-color-scale'. Definition has been changed to be more
> in the "lisp" way (thanks for help goes to #emacs). Also added a small
> description of what it does.
Ok, but the heavier cl dependency is noted below.
> * Added docstrings at some point and instructed defvar when a variable
> was candidate to customisation by users.
Good.
> * Added fix to silent byte-compilers (git-blame-file,
> git-blame-current)
Good.
> * Do not require 'cl at startup.
You removed the pop calls, but added a couple of dolist calls. So you
still need to require cl.
> * Added more informations on compatibility
Good.
> Signed-off-by: Xavier Maillard <zedek@xxxxxxx>
> ---
> contrib/emacs/git-blame.el | 71 +++++++++++++++++++++++++++----------------
> 1 files changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
> index bd87a86..6d0c1b0 100644
> --- a/contrib/emacs/git-blame.el
> +++ b/contrib/emacs/git-blame.el
> @@ -8,8 +8,8 @@
> ;; License: GPL
> ;; Keywords: git, version control, release management
> ;;
> -;; Compatibility: Emacs21
> -
> +;; Compatibility: Emacs21, Emacs22 and EmacsCVS
> +;; Git 1.5 and up
>
> ;; This file is *NOT* part of GNU Emacs.
> ;; This file is distributed under the same terms as GNU Emacs.
> @@ -61,8 +61,9 @@
>
> ;;; Compatibility:
> ;;
> -;; It requires GNU Emacs 21. If you'are using Emacs 20, try
> -;; changing this:
> +;; It requires GNU Emacs 21 or later and Git 1.5.0 and up
> +;;
> +;; If you'are using Emacs 20, try changing this:
> ;;
> ;; (overlay-put ovl 'face (list :background
> ;; (cdr (assq 'color (cddddr info)))))
> @@ -77,30 +78,43 @@
> ;;
> ;;; Code:
>
> -(require 'cl) ; to use `push', `pop'
> -
> -(defun color-scale (l)
> - (let* ((colors ())
> - r g b)
> - (setq r l)
> - (while r
> - (setq g l)
> - (while g
> - (setq b l)
> - (while b
> - (push (concat "#" (car r) (car g) (car b)) colors)
> - (pop b))
> - (pop g))
> - (pop r))
> - colors))
> +(eval-when-compile (require 'cl)) ; to use `push', `pop'
> +
> +
> +(defun git-blame-color-scale (&rest elements)
> + "Given a list, returns a list of triples formed with each
> +elements of the list.
> +
> +a b => bbb bba bab baa abb aba aaa aab"
> + (let (result)
> + (dolist (a elements)
> + (dolist (b elements)
> + (dolist (c elements)
> + (setq result (cons (format "#%s%s%s" a b c) result)))))
> + result))
> +
> +;; (git-blame-color-scale "0c" "04" "24" "1c" "2c" "34" "14" "3c") =>
> +;; ("#3c3c3c" "#3c3c14" "#3c3c34" "#3c3c2c" "#3c3c1c" "#3c3c24"
> +;; "#3c3c04" "#3c3c0c" "#3c143c" "#3c1414" "#3c1434" "#3c142c" ...)
>
> (defvar git-blame-dark-colors
> - (color-scale '("0c" "04" "24" "1c" "2c" "34" "14" "3c")))
> + (git-blame-color-scale "0c" "04" "24" "1c" "2c" "34" "14" "3c")
> + "*List of colors (format #RGB) to use in a dark environment.
> +
> +To check out the list, evaluate (list-colors-display git-blame-dark-colors).")
>
> (defvar git-blame-light-colors
> - (color-scale '("c4" "d4" "cc" "dc" "f4" "e4" "fc" "ec")))
> + (git-blame-color-scale "c4" "d4" "cc" "dc" "f4" "e4" "fc" "ec")
> + "*List of colors (format #RGB) to use in a light environment.
> +
> +To check out the list, evaluate (list-colors-display git-blame-light-colors).")
>
> -(defvar git-blame-ancient-color "dark green")
> +(defvar git-blame-colors '()
> + "Colors used by git-blame. The list is built once when activating git-blame
> +minor mode.")
> +
> +(defvar git-blame-ancient-color "dark green"
> + "*Color to be used for ancient commit.")
>
> (defvar git-blame-autoupdate t
> "*Automatically update the blame display while editing")
> @@ -125,6 +139,10 @@
> "A queue of update requests")
> (make-variable-buffer-local 'git-blame-update-queue)
>
> +;; FIXME: docstrings
> +(defvar git-blame-file nil)
> +(defvar git-blame-current nil)
> +
> (defvar git-blame-mode nil)
> (make-variable-buffer-local 'git-blame-mode)
>
> @@ -177,7 +195,7 @@ See also function `git-blame-mode'."
> "Recalculate all blame information in the current buffer"
> (interactive)
> (unless git-blame-mode
> - (error "git-blame is not active"))
> + (error "Git-blame is not active"))
>
> (git-blame-cleanup)
> (git-blame-run))
--
David Kågedal
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html