From 157e4ddfb426336abc3d1f2903a943e05576ad03 Mon Sep 17 00:00:00 2001 From: Mark Walters Date: Wed, 30 Oct 2013 18:48:10 +0000 Subject: [PATCH] Re: [PATCH 00/11] Move notmuch-tree to mainline --- 73/4e757074e6cb1530db833f67a365d0aeb1dd19 | 224 ++++++++++++++++++++++ 1 file changed, 224 insertions(+) create mode 100644 73/4e757074e6cb1530db833f67a365d0aeb1dd19 diff --git a/73/4e757074e6cb1530db833f67a365d0aeb1dd19 b/73/4e757074e6cb1530db833f67a365d0aeb1dd19 new file mode 100644 index 000000000..ee2bce2b6 --- /dev/null +++ b/73/4e757074e6cb1530db833f67a365d0aeb1dd19 @@ -0,0 +1,224 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id E2E37431FD7 + for ; Wed, 30 Oct 2013 11:48:24 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -1.098 +X-Spam-Level: +X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5 + tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001, + NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id kMBSWFLanOkN for ; + Wed, 30 Oct 2013 11:48:20 -0700 (PDT) +Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6]) + (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id E654A431FCF + for ; Wed, 30 Oct 2013 11:48:19 -0700 (PDT) +Received: from smtp.qmul.ac.uk ([138.37.6.40]) + by mail2.qmul.ac.uk with esmtp (Exim 4.71) + (envelope-from ) + id 1VbaoO-0004LH-F9; Wed, 30 Oct 2013 18:48:15 +0000 +Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost) + by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71) + (envelope-from ) + id 1VbaoO-0003gB-1p; Wed, 30 Oct 2013 18:48:12 +0000 +From: Mark Walters +To: Tomi Ollila , notmuch@notmuchmail.org +Subject: Re: [PATCH 00/11] Move notmuch-tree to mainline +In-Reply-To: +References: <1383087338-10220-1-git-send-email-markwalters1009@gmail.com> + +User-Agent: Notmuch/0.16 (http://notmuchmail.org) Emacs/23.4.1 + (x86_64-pc-linux-gnu) +Date: Wed, 30 Oct 2013 18:48:10 +0000 +Message-ID: <87d2mm8uj9.fsf@qmul.ac.uk> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Sender-Host-Address: 93.97.24.31 +X-QM-SPAM-Info: Sender has good ham record. :) +X-QM-Body-MD5: 91e31d0220c74927fff2a9c926c6b196 (of first 20000 bytes) +X-SpamAssassin-Score: 0.0 +X-SpamAssassin-SpamBar: / +X-SpamAssassin-Report: The QM spam filters have analysed this message to + determine if it is + spam. We require at least 5.0 points to mark a message as spam. + This message scored 0.0 points. Summary of the scoring: + * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail + provider * (markwalters1009[at]gmail.com) + * 0.0 AWL AWL: From: address is in the auto white-list +X-QM-Scan-Virus: ClamAV says the message is clean +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Wed, 30 Oct 2013 18:48:25 -0000 + + +Hi + +Thanks for the review + +On Wed, 30 Oct 2013, Tomi Ollila wrote: +> On Wed, Oct 30 2013, Mark Walters wrote: +> +>> This is a more complete version of the move of pick/tree to +>> mainline. The previous version was at +>> id:1382257699-29860-1-git-send-email-markwalters1009@gmail.com +> +> This series looks pretty good to me. IMO patches 1-6 are ready... +> +>> The previous version did not get the autoloading right: it all worked +>> on my setup as I had stale files around in my tree which were loaded. +>> This versions builds and tests pass at each stage on a fresh clone. +> +> You and Me seem to have different understanding what "autoload"ing means. + +Sorry about that; I just meant that it was automatically loaded when +notmuch was loaded (that is the user no longer needs an explicit +(require 'notmuch-tree) in their .emacs file. + +I would probably just stick with the require approach (to be consistent +with show etc) unless you (or others) prefer the real autoload as you +describe. + +I will fix the commit messages in the next version. + +Many thanks + +Mark + + +> To me autoloading means using (autoload ...) function. In patch 7 +> ( id:1383087338-10220-8-git-send-email-markwalters1009@gmail.com ) +> (require 'notmuch-tree) is used. +> +> To me it is fine if this approach is taken, but then calling it autoloading +> in commit message (in patches 7 and 8) is confusing. +> +> However, if (autoload ...) syntax were actually used, it could be done +> the somewhat the following way: +> +> * Don't add (require 'notmuch-tree) in notmuch.el (Patch 7) +> +> * Add (autoload 'notmuch-tree-from-search-current-query "notmuch-tree" "docstring" t) +> to notmuch.el (maybe we can live without declare-function ?) +> +> * Add (autoload 'notmuch-tree "notmuch-tree" "docstring" t) to +> notmuch-lib.el. +> +> * Now Patch 9 is good as it is now. +> +> * Don't move the functions mentioned in Patch 9 from notmuch-tree.el to +> notmuch.el but write autoload (for notmuch-tree-from-search-thread) to +> notmuch.el +> +> * Now Patch 10 is good as it is now. +> +> * Leave notmuch-tree-from-show-current-query in notmuch-tree.el, and add +> autoload for it in notmuch-show.el. Now you can also drop +> (declare-function notmuch-tree ...) out from notmuch-show.el (Patch 11). +> +> +> Tomi +> +> +>> The diffstat at the bottom of this message is huge, so I include the +>> rather more informative output from git diff -stat -M here. +>> +>> Apart from the first patch (which moves 80 lines from notmuch.el to notmuch-lib.el) the total is: +>> emacs/Makefile.local | 1 + +>> emacs/notmuch-lib.el | 1 + +>> emacs/notmuch-show.el | 10 ++++ +>> {contrib/notmuch-pick => emacs}/notmuch-tree.el | 47 ++----------------- +>> emacs/notmuch.el | 16 +++++++ +>> {contrib/notmuch-pick/test => test}/emacs-tree | 13 ----- +>> test/notmuch-test | 1 + +>> .../tree.expected-output/notmuch-tree-show-window | 0 +>> .../notmuch-tree-single-thread | 0 +>> .../tree.expected-output/notmuch-tree-tag-inbox | 0 +>> .../notmuch-tree-tag-inbox-tagged | 0 +>> .../notmuch-tree-tag-inbox-thread-tagged | 0 +>> 12 files changed, 34 insertions(+), 55 deletions(-) +>> +>> +>> Obviously this clashes with the remap/help series at +>> id:1382821479-23384-1-git-send-email-markwalters1009@gmail.com but +>> rebasing either way would be easy. +>> +>> Best wishes +>> +>> Mark +>> +>> +>> +>> +>> +>> Mark Walters (11): +>> emacs: move notmuch-help to lib +>> emacs: tree: remove unneeded declarations +>> emacs: move notmuch-tree from contrib to mainline +>> emacs: add tree to the makefile +>> test: move emacs-tree test into mainline +>> emacs: tree: remove test for emacs from tree test +>> emacs: minimal change to autoload notmuch-tree +>> test: tree: remove require from tests +>> emacs: move search based tree functions to notmuch.el +>> emacs: add z to common keymap +>> emacs: move the show entry to tree into show.el +>> +>> contrib/notmuch-pick/notmuch-tree.el | 951 -------------------- +>> contrib/notmuch-pick/test/emacs-tree | 210 ----- +>> .../tree.expected-output/notmuch-tree-show-window | 40 - +>> .../notmuch-tree-single-thread | 6 - +>> .../tree.expected-output/notmuch-tree-tag-inbox | 53 -- +>> .../notmuch-tree-tag-inbox-tagged | 53 -- +>> .../notmuch-tree-tag-inbox-thread-tagged | 53 -- +>> emacs/Makefile.local | 1 + +>> emacs/notmuch-lib.el | 88 ++ +>> emacs/notmuch-show.el | 10 + +>> emacs/notmuch-tree.el | 914 +++++++++++++++++++ +>> emacs/notmuch.el | 103 +-- +>> test/emacs-tree | 197 ++++ +>> test/notmuch-test | 1 + +>> test/tree.expected-output/notmuch-tree-show-window | 40 + +>> .../notmuch-tree-single-thread | 6 + +>> test/tree.expected-output/notmuch-tree-tag-inbox | 53 ++ +>> .../notmuch-tree-tag-inbox-tagged | 53 ++ +>> .../notmuch-tree-tag-inbox-thread-tagged | 53 ++ +>> 19 files changed, 1432 insertions(+), 1453 deletions(-) +>> delete mode 100644 contrib/notmuch-pick/notmuch-tree.el +>> delete mode 100755 contrib/notmuch-pick/test/emacs-tree +>> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-show-window +>> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-single-thread +>> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox +>> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox-tagged +>> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox-thread-tagged +>> create mode 100644 emacs/notmuch-tree.el +>> create mode 100755 test/emacs-tree +>> create mode 100644 test/tree.expected-output/notmuch-tree-show-window +>> create mode 100644 test/tree.expected-output/notmuch-tree-single-thread +>> create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox +>> create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox-tagged +>> create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox-thread-tagged +>> +>> -- +>> 1.7.9.1 +>> +>> _______________________________________________ +>> notmuch mailing list +>> notmuch@notmuchmail.org +>> http://notmuchmail.org/mailman/listinfo/notmuch -- 2.26.2