Re: [PATCH] emacs: tree/show remove duplicate function
Mark Walters markwalters1...@gmail.com writes: tree overrides notmuch-show-get-prop so that it can use many of the utility function directly. Now that tree is in mainline the version from tree can be moved to show and the original overridden show version dropped. --- Pushed. I hope there will be a followup patch making the current implicit nil return more explicit. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] emacs: tree/show remove duplicate function
Mark Walters writes: > On Tue, 15 Jul 2014, David Bremner wrote: >> Mark Walters writes: >>> + (cond ((eq major-mode 'notmuch-show-mode) >>> + (notmuch-show-get-message-properties)) >>> +((eq major-mode 'notmuch-tree-mode) >>> + (notmuch-tree-get-message-properties >> >> I see this already existed, but it looks weird to me to have a two test >> cond with no else. Is it intentional to have the code drop through and >> do nothing if neither case matches? It seems like it might be better to >> signal an error. > > I can definitely do that. But as a comparison > notmuch-search-get-result and notmuch-search-find-thread-id "work" in > any buffer in the sense of returning nil but not complaining so perhaps > the current version is more consistent. > It occurs to me that we can fix all 3 places in a followup patch, so I'll push this for now. d
[PATCH] emacs: tree/show remove duplicate function
On Tue, 15 Jul 2014, David Bremner wrote: > Mark Walters writes: >> + (cond ((eq major-mode 'notmuch-show-mode) >> + (notmuch-show-get-message-properties)) >> + ((eq major-mode 'notmuch-tree-mode) >> + (notmuch-tree-get-message-properties > > I see this already existed, but it looks weird to me to have a two test > cond with no else. Is it intentional to have the code drop through and > do nothing if neither case matches? It seems like it might be better to > signal an error. I can definitely do that. But as a comparison notmuch-search-get-result and notmuch-search-find-thread-id "work" in any buffer in the sense of returning nil but not complaining so perhaps the current version is more consistent. Plausibly a comment and an explicit nil case would be clearer for the other modes. Best wishes Mark
[PATCH] emacs: tree/show remove duplicate function
Mark Walters writes: > > Plausibly a comment and an explicit nil case would be clearer for the other > modes. > Your call. d
Re: [PATCH] emacs: tree/show remove duplicate function
On Tue, 15 Jul 2014, David Bremner da...@tethera.net wrote: Mark Walters markwalters1...@gmail.com writes: + (cond ((eq major-mode 'notmuch-show-mode) + (notmuch-show-get-message-properties)) + ((eq major-mode 'notmuch-tree-mode) + (notmuch-tree-get-message-properties I see this already existed, but it looks weird to me to have a two test cond with no else. Is it intentional to have the code drop through and do nothing if neither case matches? It seems like it might be better to signal an error. I can definitely do that. But as a comparison notmuch-search-get-result and notmuch-search-find-thread-id work in any buffer in the sense of returning nil but not complaining so perhaps the current version is more consistent. Plausibly a comment and an explicit nil case would be clearer for the other modes. Best wishes Mark ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: tree/show remove duplicate function
Mark Walters markwalters1...@gmail.com writes: Plausibly a comment and an explicit nil case would be clearer for the other modes. Your call. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: tree/show remove duplicate function
Mark Walters markwalters1...@gmail.com writes: On Tue, 15 Jul 2014, David Bremner da...@tethera.net wrote: Mark Walters markwalters1...@gmail.com writes: + (cond ((eq major-mode 'notmuch-show-mode) + (notmuch-show-get-message-properties)) +((eq major-mode 'notmuch-tree-mode) + (notmuch-tree-get-message-properties I see this already existed, but it looks weird to me to have a two test cond with no else. Is it intentional to have the code drop through and do nothing if neither case matches? It seems like it might be better to signal an error. I can definitely do that. But as a comparison notmuch-search-get-result and notmuch-search-find-thread-id work in any buffer in the sense of returning nil but not complaining so perhaps the current version is more consistent. It occurs to me that we can fix all 3 places in a followup patch, so I'll push this for now. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] emacs: tree/show remove duplicate function
Mark Walters writes: > +(cond ((eq major-mode 'notmuch-show-mode) > + (notmuch-show-get-message-properties)) > + ((eq major-mode 'notmuch-tree-mode) > + (notmuch-tree-get-message-properties I see this already existed, but it looks weird to me to have a two test cond with no else. Is it intentional to have the code drop through and do nothing if neither case matches? It seems like it might be better to signal an error. d
Re: [PATCH] emacs: tree/show remove duplicate function
Mark Walters markwalters1...@gmail.com writes: +(cond ((eq major-mode 'notmuch-show-mode) + (notmuch-show-get-message-properties)) + ((eq major-mode 'notmuch-tree-mode) + (notmuch-tree-get-message-properties I see this already existed, but it looks weird to me to have a two test cond with no else. Is it intentional to have the code drop through and do nothing if neither case matches? It seems like it might be better to signal an error. d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] emacs: tree/show remove duplicate function
On Sat, Jun 07 2014, Mark Walters wrote:
> tree overrides notmuch-show-get-prop so that it can use many of the
> utility function directly. Now that tree is in mainline the version
> from tree can be moved to show and the original overridden show
> version dropped.
> ---
> I should have done this ages ago but forgot about it.
LGTM, and
https://github.com/domo141/nottoomuch/blob/master/make-one-notmuch-el.pl
no longer complains about duplicate function :D
>
> Best wishes
>
> Mark
Tomi
>
>
> emacs/notmuch-show.el | 12 +++-
> emacs/notmuch-tree.el | 16
> 2 files changed, 11 insertions(+), 17 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index 10fc872..b922a38 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -46,6 +46,7 @@
> (declare-function notmuch-save-attachments "notmuch" (mm-handle
> queryp))
> (declare-function notmuch-tree "notmuch-tree"
> ( query query-context target buffer-name
> open-target))
> +(declare-function notmuch-tree-get-message-properties "notmuch-tree" nil)
>
> (defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
>"Headers that should be shown in a message, in this order.
> @@ -1459,8 +1460,17 @@ (defun notmuch-show-set-prop (prop val props)
> (notmuch-show-set-message-properties props)))
>
> (defun notmuch-show-get-prop (prop props)
> + "Get property PROP from current message in show or tree mode.
> +
> +It gets property PROP from PROPS or, if PROPS is nil, the current
> +message in either tree or show. This means that several utility
> +functions in notmuch-show can be used directly by notmuch-tree as
> +they just need the correct message properties."
>(let ((props (or props
> -(notmuch-show-get-message-properties
> +(cond ((eq major-mode 'notmuch-show-mode)
> + (notmuch-show-get-message-properties))
> + ((eq major-mode 'notmuch-tree-mode)
> + (notmuch-tree-get-message-properties))
> (plist-get props prop)))
>
> (defun notmuch-show-get-message-id ( bare)
> diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
> index 7d5f475..88f92f2 100644
> --- a/emacs/notmuch-tree.el
> +++ b/emacs/notmuch-tree.el
> @@ -290,22 +290,6 @@ (defun notmuch-tree-get-message-properties ()
> (beginning-of-line)
> (get-text-property (point) :notmuch-message-properties)))
>
> -;; XXX This should really be a lib function but we are trying to
> -;; reduce impact on the code base.
> -(defun notmuch-show-get-prop (prop props)
> - "This is a tree view overridden version of notmuch-show-get-prop
> -
> -It gets property PROP from PROPS or, if PROPS is nil, the current
> -message in either tree or show. This means that several functions
> -in notmuch-show now work unchanged in tree as they just need the
> -correct message properties."
> - (let ((props (or props
> -(cond ((eq major-mode 'notmuch-show-mode)
> - (notmuch-show-get-message-properties))
> - ((eq major-mode 'notmuch-tree-mode)
> - (notmuch-tree-get-message-properties))
> -(plist-get props prop)))
> -
> (defun notmuch-tree-set-message-properties (props)
>(save-excursion
> (beginning-of-line)
> --
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] emacs: tree/show remove duplicate function
tree overrides notmuch-show-get-prop so that it can use many of the
utility function directly. Now that tree is in mainline the version
from tree can be moved to show and the original overridden show
version dropped.
---
I should have done this ages ago but forgot about it.
Best wishes
Mark
emacs/notmuch-show.el | 12 +++-
emacs/notmuch-tree.el | 16
2 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 10fc872..b922a38 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -46,6 +46,7 @@
(declare-function notmuch-save-attachments "notmuch" (mm-handle
queryp))
(declare-function notmuch-tree "notmuch-tree"
( query query-context target buffer-name
open-target))
+(declare-function notmuch-tree-get-message-properties "notmuch-tree" nil)
(defcustom notmuch-message-headers '("Subject" "To" "Cc" "Date")
"Headers that should be shown in a message, in this order.
@@ -1459,8 +1460,17 @@ (defun notmuch-show-set-prop (prop val props)
(notmuch-show-set-message-properties props)))
(defun notmuch-show-get-prop (prop props)
+ "Get property PROP from current message in show or tree mode.
+
+It gets property PROP from PROPS or, if PROPS is nil, the current
+message in either tree or show. This means that several utility
+functions in notmuch-show can be used directly by notmuch-tree as
+they just need the correct message properties."
(let ((props (or props
- (notmuch-show-get-message-properties
+ (cond ((eq major-mode 'notmuch-show-mode)
+ (notmuch-show-get-message-properties))
+((eq major-mode 'notmuch-tree-mode)
+ (notmuch-tree-get-message-properties))
(plist-get props prop)))
(defun notmuch-show-get-message-id ( bare)
diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index 7d5f475..88f92f2 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -290,22 +290,6 @@ (defun notmuch-tree-get-message-properties ()
(beginning-of-line)
(get-text-property (point) :notmuch-message-properties)))
-;; XXX This should really be a lib function but we are trying to
-;; reduce impact on the code base.
-(defun notmuch-show-get-prop (prop props)
- "This is a tree view overridden version of notmuch-show-get-prop
-
-It gets property PROP from PROPS or, if PROPS is nil, the current
-message in either tree or show. This means that several functions
-in notmuch-show now work unchanged in tree as they just need the
-correct message properties."
- (let ((props (or props
- (cond ((eq major-mode 'notmuch-show-mode)
- (notmuch-show-get-message-properties))
-((eq major-mode 'notmuch-tree-mode)
- (notmuch-tree-get-message-properties))
-(plist-get props prop)))
-
(defun notmuch-tree-set-message-properties (props)
(save-excursion
(beginning-of-line)
--
1.7.10.4
[PATCH] emacs: tree/show remove duplicate function
tree overrides notmuch-show-get-prop so that it can use many of the utility function directly. Now that tree is in mainline the version from tree can be moved to show and the original overridden show version dropped. --- I should have done this ages ago but forgot about it. Best wishes Mark emacs/notmuch-show.el | 12 +++- emacs/notmuch-tree.el | 16 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 10fc872..b922a38 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -46,6 +46,7 @@ (declare-function notmuch-save-attachments notmuch (mm-handle optional queryp)) (declare-function notmuch-tree notmuch-tree (optional query query-context target buffer-name open-target)) +(declare-function notmuch-tree-get-message-properties notmuch-tree nil) (defcustom notmuch-message-headers '(Subject To Cc Date) Headers that should be shown in a message, in this order. @@ -1459,8 +1460,17 @@ (defun notmuch-show-set-prop (prop val optional props) (notmuch-show-set-message-properties props))) (defun notmuch-show-get-prop (prop optional props) + Get property PROP from current message in show or tree mode. + +It gets property PROP from PROPS or, if PROPS is nil, the current +message in either tree or show. This means that several utility +functions in notmuch-show can be used directly by notmuch-tree as +they just need the correct message properties. (let ((props (or props - (notmuch-show-get-message-properties + (cond ((eq major-mode 'notmuch-show-mode) + (notmuch-show-get-message-properties)) +((eq major-mode 'notmuch-tree-mode) + (notmuch-tree-get-message-properties)) (plist-get props prop))) (defun notmuch-show-get-message-id (optional bare) diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el index 7d5f475..88f92f2 100644 --- a/emacs/notmuch-tree.el +++ b/emacs/notmuch-tree.el @@ -290,22 +290,6 @@ (defun notmuch-tree-get-message-properties () (beginning-of-line) (get-text-property (point) :notmuch-message-properties))) -;; XXX This should really be a lib function but we are trying to -;; reduce impact on the code base. -(defun notmuch-show-get-prop (prop optional props) - This is a tree view overridden version of notmuch-show-get-prop - -It gets property PROP from PROPS or, if PROPS is nil, the current -message in either tree or show. This means that several functions -in notmuch-show now work unchanged in tree as they just need the -correct message properties. - (let ((props (or props - (cond ((eq major-mode 'notmuch-show-mode) - (notmuch-show-get-message-properties)) -((eq major-mode 'notmuch-tree-mode) - (notmuch-tree-get-message-properties)) -(plist-get props prop))) - (defun notmuch-tree-set-message-properties (props) (save-excursion (beginning-of-line) -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: tree/show remove duplicate function
On Sat, Jun 07 2014, Mark Walters markwalters1...@gmail.com wrote: tree overrides notmuch-show-get-prop so that it can use many of the utility function directly. Now that tree is in mainline the version from tree can be moved to show and the original overridden show version dropped. --- I should have done this ages ago but forgot about it. LGTM, and https://github.com/domo141/nottoomuch/blob/master/make-one-notmuch-el.pl no longer complains about duplicate function :D Best wishes Mark Tomi emacs/notmuch-show.el | 12 +++- emacs/notmuch-tree.el | 16 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index 10fc872..b922a38 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -46,6 +46,7 @@ (declare-function notmuch-save-attachments notmuch (mm-handle optional queryp)) (declare-function notmuch-tree notmuch-tree (optional query query-context target buffer-name open-target)) +(declare-function notmuch-tree-get-message-properties notmuch-tree nil) (defcustom notmuch-message-headers '(Subject To Cc Date) Headers that should be shown in a message, in this order. @@ -1459,8 +1460,17 @@ (defun notmuch-show-set-prop (prop val optional props) (notmuch-show-set-message-properties props))) (defun notmuch-show-get-prop (prop optional props) + Get property PROP from current message in show or tree mode. + +It gets property PROP from PROPS or, if PROPS is nil, the current +message in either tree or show. This means that several utility +functions in notmuch-show can be used directly by notmuch-tree as +they just need the correct message properties. (let ((props (or props -(notmuch-show-get-message-properties +(cond ((eq major-mode 'notmuch-show-mode) + (notmuch-show-get-message-properties)) + ((eq major-mode 'notmuch-tree-mode) + (notmuch-tree-get-message-properties)) (plist-get props prop))) (defun notmuch-show-get-message-id (optional bare) diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el index 7d5f475..88f92f2 100644 --- a/emacs/notmuch-tree.el +++ b/emacs/notmuch-tree.el @@ -290,22 +290,6 @@ (defun notmuch-tree-get-message-properties () (beginning-of-line) (get-text-property (point) :notmuch-message-properties))) -;; XXX This should really be a lib function but we are trying to -;; reduce impact on the code base. -(defun notmuch-show-get-prop (prop optional props) - This is a tree view overridden version of notmuch-show-get-prop - -It gets property PROP from PROPS or, if PROPS is nil, the current -message in either tree or show. This means that several functions -in notmuch-show now work unchanged in tree as they just need the -correct message properties. - (let ((props (or props -(cond ((eq major-mode 'notmuch-show-mode) - (notmuch-show-get-message-properties)) - ((eq major-mode 'notmuch-tree-mode) - (notmuch-tree-get-message-properties)) -(plist-get props prop))) - (defun notmuch-tree-set-message-properties (props) (save-excursion (beginning-of-line) -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
