Return-Path: <markwalters1009@gmail.com>
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 95B2B431FC7
	for <notmuch@notmuchmail.org>; Wed, 21 May 2014 02:59:09 -0700 (PDT)
X-Virus-Scanned: Debian amavisd-new at olra.theworths.org
X-Spam-Flag: NO
X-Spam-Score: 0.201
X-Spam-Level: 
X-Spam-Status: No, score=0.201 tagged_above=-999 required=5
	tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,
	FREEMAIL_ENVFROM_END_DIGIT=1, FREEMAIL_FROM=0.001,
	RCVD_IN_DNSWL_LOW=-0.7] 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 YYHDEm9qyWvN for <notmuch@notmuchmail.org>;
	Wed, 21 May 2014 02:59:04 -0700 (PDT)
Received: from mail-wi0-f176.google.com (mail-wi0-f176.google.com
	[209.85.212.176]) (using TLSv1 with cipher RC4-SHA (128/128 bits))
	(No client certificate requested)
	by olra.theworths.org (Postfix) with ESMTPS id E469F431FC4
	for <notmuch@notmuchmail.org>; Wed, 21 May 2014 02:59:03 -0700 (PDT)
Received: by mail-wi0-f176.google.com with SMTP id n15so7360239wiw.3
	for <notmuch@notmuchmail.org>; Wed, 21 May 2014 02:59:01 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;
	h=from:to:cc:subject:date:message-id:in-reply-to:references;
	bh=UNWEXWTz/XV+fy3Enf5s+AntmoVmdr0PRCeLNzbJz08=;
	b=pLrOHbAHE1p9gWirIFTrSwY63SSP4pT7CXrCTmDOTaW29XAzPR3Zwugf22merJBltm
	BezFgST38/AlvLUwRv1mGoGYXobpk3yCUs97UFycHoAsB4t9KZ1pWYdldU+MNqOCh1mv
	GL3JVfXY6mszNKzY3ayaEdiA34ea7RjHBshX2dEwu5gf5m6ZgDs5JDeZ3N93gDuPRNpN
	ss3hNcVFbtHtUOymYpuHfdxhlyOAQGmbouEjWLecvnhTgT5PnhYSRCXkas+//yviW3rh
	VlYtxI9SXx9ONe2a0QFpx7XB6icbb4/GkA0XNLC4A0LYEJjGbA33TM5wwfGpBOgW3wF1
	7ayA==
X-Received: by 10.194.221.69 with SMTP id qc5mr800900wjc.91.1400666341522;
	Wed, 21 May 2014 02:59:01 -0700 (PDT)
Received: from localhost (5751dfa2.skybroadband.com. [87.81.223.162])
	by mx.google.com with ESMTPSA id
	kp5sm21930823wjb.30.2014.05.21.02.59.00 for <multiple recipients>
	(version=TLSv1.2 cipher=RC4-SHA bits=128/128);
	Wed, 21 May 2014 02:59:00 -0700 (PDT)
From: Mark Walters <markwalters1009@gmail.com>
To: notmuch@notmuchmail.org,
	David Edmondson <dme@dme.org>
Subject: [PATCH] emacs: make sure tagging on an empty query is harmless
Date: Wed, 21 May 2014 10:58:50 +0100
Message-Id: <1400666330-4363-1-git-send-email-markwalters1009@gmail.com>
X-Mailer: git-send-email 1.7.10.4
In-Reply-To: <87iop0ynl2.fsf@qmul.ac.uk>
References: <87iop0ynl2.fsf@qmul.ac.uk>
X-BeenThere: notmuch@notmuchmail.org
X-Mailman-Version: 2.1.13
Precedence: list
List-Id: "Use and development of the notmuch mail system."
	<notmuch.notmuchmail.org>
List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>
List-Archive: <http://notmuchmail.org/pipermail/notmuch>
List-Post: <mailto:notmuch@notmuchmail.org>
List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>
List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,
	<mailto:notmuch-request@notmuchmail.org?subject=subscribe>
X-List-Received-Date: Wed, 21 May 2014 09:59:09 -0000

Currently notmuch-tag throws a "wrong-type-argument stringp nil" if
passed a nil query-string. Catch this and provide a more useful error
message. This fixes a case in notmuch-tree (if you try to tag when at
the end of the buffer).

Secondly, as pointed out by David (dme)
`notmuch-search-find-stable-query-region' can return the query string
() if there are no messages in the region. This gets passed to notmuch
tag, and due to interactions in the optimize_query code in
notmuch-tag.c becomes, in the case tag-change is -inbox, "( () ) and
(tag:inbox)". This query matches some strange collection of messages
which then get archived. This should probably be fixed, but in any
case make `notmuch-search-find-stable-query-region' return a nil
query-string in this case.

This avoids data-loss (random tag removal) in this case.
---

This is my attempt to solve the same problem as the parent. I prefer
not throwing an error in n.s.f.s.q.r as it is difficult for the caller
to catch cleanly. Throwing it in notmuch-tag is fine as the caller can
trivially check for query-string being nil before calling notmuch-tag
if it wants to deal with it gracefully.

If people do prefer an error in n.s.f.s.q.r as in the parent patch
then I think we should update the error message. The first hunk of
this should also be applied to catch nil queries to notmuch-tag gracefully.

Although this has been present for a while I think it is a dataloss
issue so a fix should go in for 0.18.1

Best wishes

Mark




 emacs/notmuch-tag.el |    2 ++
 emacs/notmuch.el     |    6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-tag.el b/emacs/notmuch-tag.el
index 07c260e..f54aa9d 100644
--- a/emacs/notmuch-tag.el
+++ b/emacs/notmuch-tag.el
@@ -387,6 +387,8 @@ (defun notmuch-tag (query tag-changes)
 	  (unless (string-match-p "^[-+]\\S-+$" tag-change)
 	    (error "Tag must be of the form `+this_tag' or `-that_tag'")))
 	tag-changes)
+  (unless query
+    (error "Nothing to tag!"))
   (unless (null tag-changes)
     (run-hooks 'notmuch-before-tag-hook)
     (if (<= (length query) notmuch-tag-argument-limit)
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 6c0bc1b..1adea9c 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -428,14 +428,16 @@ (defun notmuch-search-find-stable-query-region (beg end &optional only-matched)
   "Return the stable query for the current region.
 
 If ONLY-MATCHED is non-nil, include only matched messages.  If it
-is nil, include both matched and unmatched messages."
+is nil, include both matched and unmatched messages. If there are
+no messages in the region then return nil."
   (let ((query-list nil) (all (not only-matched)))
     (dolist (queries (notmuch-search-properties-in-region :query beg end))
       (when (first queries)
 	(push (first queries) query-list))
       (when (and all (second queries))
 	(push (second queries) query-list)))
-    (concat "(" (mapconcat 'identity query-list ") or (") ")")))
+    (when query-list
+      (concat "(" (mapconcat 'identity query-list ") or (") ")"))))
 
 (defun notmuch-search-find-authors ()
   "Return the authors for the current thread"
-- 
1.7.10.4