unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] emacs: fix notmuch-search-line-faces defcustom
@ 2016-10-15  9:40 Mark Walters
  2016-10-17 11:53 ` David Bremner
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Mark Walters @ 2016-10-15  9:40 UTC (permalink / raw)
  To: notmuch

In commit 2a7b11b064233afc4feead876fa396e3c18a6b91 the default value
for notmuch-search-line-faces was changed so that it didn't match the
specification in the corresponding defcustom. This meant that it was
difficult for the user to customize this variable as they got a type
mismatch error.

Note anyone who had already customised this variable would not see
this bug as their customisation would match the defcustom.
---

Getting this defcustom to work was quite tricky, so please could
people test: just try M-x customize-variable notmuch-search-line-faces
and see if a custom widget displays correctly (in particular that it
doesn't complaine about a type mismatch.

For testing devel/try-emacs-mua -q (or -Q) is useful (which seems to
be fine) so I am particularly interested in whether people's existsing
customisations break the defcustom widget.

I could not persuade the custom-edit-face widget to respect the :tag
property so had to do create a new widget. Anyway, this seems to fix
the defcustom so that the new default value, and previously customised
values all work.

I am not certain this is better than just reverting 2a7b -- even with
the above the customisation for notmuch-search-line-faces is more
complex (to the user as well as in the code)

Finally, the docstring says that a list of faces is also allowed. I
believe that is true but making that work with the customize widget
would be horrible. I think we should remove the documentation for it
from the defcustom and just put a lisp comment in the source code --
if the user wants to set it with a "setq" then they can read the
actual source.

Best wishes

Mark



emacs/notmuch.el | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 2834d44..d00c21a 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -670,9 +670,16 @@ of the result."
 		  (goto-char (point-min))
 		  (forward-line (1- notmuch-search-target-line)))))))))
 
+(define-widget 'notmuch--custom-face-edit 'lazy
+  "Custom face edit with a tag Edit Face"
+  ;; I could not persuage custom-face-edit to respect the :tag
+  ;; property so create a widget specially
+  :tag "Manually specify face"
+  :type 'custom-face-edit)
+
 (defcustom notmuch-search-line-faces
-  '(("unread" 'notmuch-search-unread-face)
-    ("flagged" 'notmuch-search-flagged-face))
+  '(("unread" . notmuch-search-unread-face)
+    ("flagged" . notmuch-search-flagged-face))
   "Alist of tags to faces for line highlighting in notmuch-search.
 Each element looks like (TAG . FACE).
 A thread with TAG will have FACE applied.
@@ -690,7 +697,9 @@ matching tags are merged, with earlier attributes overriding
 later. A message having both \"deleted\" and \"unread\" tags with
 the above settings would have a green foreground and blue
 background."
-  :type '(alist :key-type (string) :value-type (custom-face-edit))
+  :type '(alist :key-type (string)
+		:value-type (radio (face :tag "Face name")
+				    (notmuch--custom-face-edit)))
   :group 'notmuch-search
   :group 'notmuch-faces)
 
-- 
2.1.4

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

* Re: [PATCH] emacs: fix notmuch-search-line-faces defcustom
  2016-10-15  9:40 [PATCH] emacs: fix notmuch-search-line-faces defcustom Mark Walters
@ 2016-10-17 11:53 ` David Bremner
  2016-10-19 11:49 ` David Bremner
  2016-10-19 22:33 ` [PATCH] emacs: fix notmuch-search-line-faces defcustom Matt Armstrong
  2 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2016-10-17 11:53 UTC (permalink / raw)
  To: Mark Walters, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

> Getting this defcustom to work was quite tricky, so please could
> people test: just try M-x customize-variable notmuch-search-line-faces
> and see if a custom widget displays correctly (in particular that it
> doesn't complaine about a type mismatch.

it works for me, fwiw

> I am not certain this is better than just reverting 2a7b -- even with
> the above the customisation for notmuch-search-line-faces is more
> complex (to the user as well as in the code)

I think I can live with the complexity from a UI point of view. I agree
it's a bit aesthetically displeasing, but I don't think it prevents the
user from getting the face customized.

> Finally, the docstring says that a list of faces is also allowed. I
> believe that is true but making that work with the customize widget
> would be horrible. I think we should remove the documentation for it
> from the defcustom and just put a lisp comment in the source code --
> if the user wants to set it with a "setq" then they can read the
> actual source.

OK; this could be a followup patch.

I'll wait a few more days for discussion and testing.

d

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

* Re: [PATCH] emacs: fix notmuch-search-line-faces defcustom
  2016-10-15  9:40 [PATCH] emacs: fix notmuch-search-line-faces defcustom Mark Walters
  2016-10-17 11:53 ` David Bremner
@ 2016-10-19 11:49 ` David Bremner
  2016-10-22  6:45   ` [PATCH] NEWS for two bugfixes Mark Walters
  2016-10-19 22:33 ` [PATCH] emacs: fix notmuch-search-line-faces defcustom Matt Armstrong
  2 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2016-10-19 11:49 UTC (permalink / raw)
  To: Mark Walters, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

> In commit 2a7b11b064233afc4feead876fa396e3c18a6b91 the default value
> for notmuch-search-line-faces was changed so that it didn't match the
> specification in the corresponding defcustom. This meant that it was
> difficult for the user to customize this variable as they got a type
> mismatch error.
>
> Note anyone who had already customised this variable would not see
> this bug as their customisation would match the defcustom.

pushed this (and the other 2a7b1 fix) to release and master. Mark, would
you mind writing a short NEWS entry for 0.23.1?

d

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

* Re: [PATCH] emacs: fix notmuch-search-line-faces defcustom
  2016-10-15  9:40 [PATCH] emacs: fix notmuch-search-line-faces defcustom Mark Walters
  2016-10-17 11:53 ` David Bremner
  2016-10-19 11:49 ` David Bremner
@ 2016-10-19 22:33 ` Matt Armstrong
  2 siblings, 0 replies; 6+ messages in thread
From: Matt Armstrong @ 2016-10-19 22:33 UTC (permalink / raw)
  To: Mark Walters, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

> In commit 2a7b11b064233afc4feead876fa396e3c18a6b91 the default value
> for notmuch-search-line-faces was changed so that it didn't match the
> specification in the corresponding defcustom. This meant that it was
> difficult for the user to customize this variable as they got a type
> mismatch error.

Mark, thanks for fixing the defcustom that I broke.  :-) I have verified
that it now works for me.  This testing induced me to zero-out my
notmuch face customizations, which reminded me that the defaults are bad
in places for Emacs themes with dark backgrounds.  I'll send a patch for
that.

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

* [PATCH] NEWS for two bugfixes
  2016-10-19 11:49 ` David Bremner
@ 2016-10-22  6:45   ` Mark Walters
  2016-10-22 15:50     ` David Bremner
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Walters @ 2016-10-22  6:45 UTC (permalink / raw)
  To: notmuch

This adds news items for the two bugs

    emacs: search face bugfix
and
    emacs: fix notmuch-search-line-faces defcustom
---

Hi here is a NEWS update as requested.

Best wishes

Mark


NEWS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/NEWS b/NEWS
index ac3ceb1..547b961 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,15 @@ Require Xapian >= 1.2.6
 Emacs
 -----
 
+Fix default colours for unread and flagged messages
+
+  In 0.23 the default colours for unread and flagged messages in
+  search view were accidentally swapped. This release returns them to
+  the original colours.
+
+  A related change in 0.23 broke the customize widget for
+  notmuch-search-line-faces. This is now fixed.
+
 Fix test failure with Emacs 25.1
 
   A previously undiscovered jit-lock related bug was exposed by Emacs
-- 
2.1.4

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

* Re: [PATCH] NEWS for two bugfixes
  2016-10-22  6:45   ` [PATCH] NEWS for two bugfixes Mark Walters
@ 2016-10-22 15:50     ` David Bremner
  0 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2016-10-22 15:50 UTC (permalink / raw)
  To: Mark Walters, notmuch

Mark Walters <markwalters1009@gmail.com> writes:

> This adds news items for the two bugs
>
>     emacs: search face bugfix
> and
>     emacs: fix notmuch-search-line-faces defcustom
> ---
>
> Hi here is a NEWS update as requested.

pushed to release and master

d

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

end of thread, other threads:[~2016-10-22 15:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-15  9:40 [PATCH] emacs: fix notmuch-search-line-faces defcustom Mark Walters
2016-10-17 11:53 ` David Bremner
2016-10-19 11:49 ` David Bremner
2016-10-22  6:45   ` [PATCH] NEWS for two bugfixes Mark Walters
2016-10-22 15:50     ` David Bremner
2016-10-19 22:33 ` [PATCH] emacs: fix notmuch-search-line-faces defcustom Matt Armstrong

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

	https://yhetil.org/notmuch.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).