* [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
* [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] 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
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).