bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#10181: 24.0.92; [wishlist] split `diff-refine-change' in several fac


From: Stefan Monnier
Subject: bug#10181: 24.0.92; [wishlist] split `diff-refine-change' in several faces
Date: Sun, 20 May 2012 11:06:38 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1.50 (gnu/linux)

>  (defface diff-removed
> -  '((t :inherit diff-changed))
> +  '((((class color) (min-colors 88))
> +     :background "#ffdddd")
> +    (((class color))
> +     :foreground "red"))

Please keep the inheritance, even if the default overrides all fields.

> +                        ((not (or (get diff-removed-face 'customized-face)
> +                                  (get diff-removed-face 'saved-face)
> +                                  (get diff-added-face 'customized-face)
> +                                  (get diff-added-face 'saved-face)))

Making decisions based on whether something is customized or not sounds
fundamentally wrong.
Maybe you meant something more like:

           ((and (face-equal diff-changed-face diff-added-face)
                 (face-equal diff-changed-face diff-removed-face))

Tho, I don't see what's the point of this `changed' color-scheme: you
get the same visual result if you use the `removed-added' color scheme
and pretty much the same resource usage.  The only point of the
distinction between removed-added and changed-removed-added is to avoid
the additional resource usage when it would have no visual effect.

> +                        ((or (get diff-changed-face 'customized-face)
> +                             (get diff-changed-face 'saved-face))
> +                         'changed-removed-added)

This is very wrong

> +(defface diff-refine-removed
> +  '((((class color) (min-colors 88))
> +     :background "#ffaaaa")
> +    (t :inherit diff-removed :inverse-video t))
> +  "Face used for removed characters shown by `diff-refine-hunk'."
> +  :group 'diff-mode
> +  :version "24.2")
> +
> +(defface diff-refine-added
> +  '((((class color) (min-colors 88))
> +     :background "#aaffaa")
> +    (t :inherit diff-added :inverse-video t))
> +  "Face used for added characters shown by `diff-refine-hunk'."
> +  :group 'diff-mode
> +  :version "24.2")

This is just an accidental left-over, right?


        Stefan





reply via email to

[Prev in Thread] Current Thread [Next in Thread]