Re: [PATCH] git-blame.el: pick a set of random colors for each git-blame turn

Xavier Maillard [off-list ref] writes:

I thought it would be cool to have different set of colors for each
git-blame-mode. Function `git-blame-new-commit' does this for us
picking when possible, a random colors based on the set we build on
startup. When it fails, `git-blame-ancient-color' will be used. We
also take care not to use the same color more than once (thank you
David Kågedal).

Closer, but still no cigar :-)

 (defun git-blame-new-commit (hash src-line res-line num-lines)
   (save-excursion
     (set-buffer git-blame-file)
     (let ((info (gethash hash git-blame-cache))
           (inhibit-point-motion-hooks t)
-          (inhibit-modification-hooks t))
+          (inhibit-modification-hooks t)
+	  (colors git-blame-colors))
       (when (not info)
-        (let ((color (pop git-blame-colors)))
-          (unless color
-            (setq color git-blame-ancient-color))
-          (setq info (list hash src-line res-line num-lines
+	;; Assign a random color to each new commit info
+	;; Take care not to select the same color multiple times
+	(let ((color (if colors
+			 (git-blame-random-pop colors)
+		       git-blame-ancient-color)))
+	  (setq info (list hash src-line res-line num-lines
                            (git-describe-commit hash)
                            (cons 'color color))))
         (puthash hash info git-blame-cache))

You are still making a copy of the list head pointer (colors ->
git-blame-colors), and then you do (git-blame-random-pop colors).
This will not update git-blame-colors if the first element was popped,
which means that you will keep reusing that color.  Since you really
do want to always update the buffer-local git-blame-colors, I don't
see why you bind a local variable and work with that instead.

And the last diff line is whitespace-only.  You replaced eight spaces
with a TAB.

-- 
David Kågedal