unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#26969: 26.0.50; diff-changed invisible by default
@ 2017-05-17 19:12 Stefan Monnier
  2017-05-22 20:19 ` Juri Linkov
  2020-08-24 15:10 ` Lars Ingebrigtsen
  0 siblings, 2 replies; 5+ messages in thread
From: Stefan Monnier @ 2017-05-17 19:12 UTC (permalink / raw)
  To: 26969; +Cc: Justin Burkett

Package: Emacs
Version: 26.0.50

Nowadays the `diff-changed` face is defined to be "invisible" (i.e. the
text is displayed identically with or without that face).
IOW it can only be used as a parent of diff-added or diff-removed (or
some other such face).

This is a bad idea since in cases such as vdiff.el where some changes
are neither additions nor removals, it would make sense to highlight the
buffer as just "changed" and using that face is a natural choice.

I suggest the patch below for that.  It is designed such that it should
make no visible difference to users of diff-added and diff-removed since
these faces override the parts that are set by my patch in diff-changed.

Another option would be to introduce a new face (not sure how to call
it, maybe diff-unspecified-change, maybe?) which inherits from
diff-changed as well.


        Stefan


diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index aa8d77882e..1aa6c05662 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -290,9 +290,14 @@ diff-added
   :group 'diff-mode)
 
 (defface diff-changed
-  '((t nil))
+  '((((class color) (min-colors 88) (background light))
+     :background "grey90")
+    (((class color) (min-colors 88) (background dark))
+     :background "grey20")
+    (((class color))
+     :foreground "grey"))
   "`diff-mode' face used to highlight changed lines."
-  :version "25.1"
+  :version "26.1"
   :group 'diff-mode)
 
 (defface diff-indicator-removed





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#26969: 26.0.50; diff-changed invisible by default
  2017-05-17 19:12 bug#26969: 26.0.50; diff-changed invisible by default Stefan Monnier
@ 2017-05-22 20:19 ` Juri Linkov
  2017-05-22 22:10   ` Stefan Monnier
  2020-08-24 15:10 ` Lars Ingebrigtsen
  1 sibling, 1 reply; 5+ messages in thread
From: Juri Linkov @ 2017-05-22 20:19 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 26969, Justin Burkett

> Nowadays the `diff-changed` face is defined to be "invisible" (i.e. the
> text is displayed identically with or without that face).
> IOW it can only be used as a parent of diff-added or diff-removed (or
> some other such face).
>
> This is a bad idea since in cases such as vdiff.el where some changes
> are neither additions nor removals, it would make sense to highlight the
> buffer as just "changed" and using that face is a natural choice.
>
> I suggest the patch below for that.  It is designed such that it should
> make no visible difference to users of diff-added and diff-removed since
> these faces override the parts that are set by my patch in diff-changed.

This might break a fragile condition in ‘diff-use-changed-face’
that was created to support backward-compatibility of previously
customized faces.  But maybe it's time to simplify the logic
of diff face definitions.

> Another option would be to introduce a new face (not sure how to call
> it, maybe diff-unspecified-change, maybe?) which inherits from
> diff-changed as well.

Then authors of external packages have to wait for the next Emacs release
to start using it.

>  (defface diff-changed
> -  '((t nil))
> +  '((((class color) (min-colors 88) (background light))
> +     :background "grey90")
> +    (((class color) (min-colors 88) (background dark))
> +     :background "grey20")
> +    (((class color))
> +     :foreground "grey"))

I wonder why grey?  It used to be yellow.





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#26969: 26.0.50; diff-changed invisible by default
  2017-05-22 20:19 ` Juri Linkov
@ 2017-05-22 22:10   ` Stefan Monnier
  0 siblings, 0 replies; 5+ messages in thread
From: Stefan Monnier @ 2017-05-22 22:10 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 26969, Justin Burkett

>> (defface diff-changed
>> -  '((t nil))
>> +  '((((class color) (min-colors 88) (background light))
>> +     :background "grey90")
>> +    (((class color) (min-colors 88) (background dark))
>> +     :background "grey20")
>> +    (((class color))
>> +     :foreground "grey"))

> I wonder why grey?  It used to be yellow.

Grey is beautiful ;-)


        Stefan "who doesn't really care about colors, so long as they
                lie on the white->black axis"





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#26969: 26.0.50; diff-changed invisible by default
  2017-05-17 19:12 bug#26969: 26.0.50; diff-changed invisible by default Stefan Monnier
  2017-05-22 20:19 ` Juri Linkov
@ 2020-08-24 15:10 ` Lars Ingebrigtsen
  2021-08-13 13:37   ` Lars Ingebrigtsen
  1 sibling, 1 reply; 5+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-24 15:10 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 26969, Justin Burkett

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Nowadays the `diff-changed` face is defined to be "invisible" (i.e. the
> text is displayed identically with or without that face).
> IOW it can only be used as a parent of diff-added or diff-removed (or
> some other such face).
>
> This is a bad idea since in cases such as vdiff.el where some changes
> are neither additions nor removals, it would make sense to highlight the
> buffer as just "changed" and using that face is a natural choice.
>
> I suggest the patch below for that.  It is designed such that it should
> make no visible difference to users of diff-added and diff-removed since
> these faces override the parts that are set by my patch in diff-changed.

That does sound a bit fragile...

> Another option would be to introduce a new face (not sure how to call
> it, maybe diff-unspecified-change, maybe?) which inherits from
> diff-changed as well.

I think that sounds more straightforward?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#26969: 26.0.50; diff-changed invisible by default
  2020-08-24 15:10 ` Lars Ingebrigtsen
@ 2021-08-13 13:37   ` Lars Ingebrigtsen
  0 siblings, 0 replies; 5+ messages in thread
From: Lars Ingebrigtsen @ 2021-08-13 13:37 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 26969, Justin Burkett

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> Another option would be to introduce a new face (not sure how to call
>> it, maybe diff-unspecified-change, maybe?) which inherits from
>> diff-changed as well.
>
> I think that sounds more straightforward?

So I've now done this in Emacs 28.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-08-13 13:37 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-17 19:12 bug#26969: 26.0.50; diff-changed invisible by default Stefan Monnier
2017-05-22 20:19 ` Juri Linkov
2017-05-22 22:10   ` Stefan Monnier
2020-08-24 15:10 ` Lars Ingebrigtsen
2021-08-13 13:37   ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).