From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id GIAGJUgdR2O4DwAAbAwnHQ (envelope-from ) for ; Wed, 12 Oct 2022 22:02:16 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id eKomJUgdR2Ph9gAAauVa8A (envelope-from ) for ; Wed, 12 Oct 2022 22:02:16 +0200 Received: from mail.notmuchmail.org (yantan.tethera.net [135.181.149.255]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 46F1E3CBC6 for ; Wed, 12 Oct 2022 22:02:16 +0200 (CEST) Received: from yantan.tethera.net (localhost [127.0.0.1]) by mail.notmuchmail.org (Postfix) with ESMTP id E0AE75F37A; Wed, 12 Oct 2022 20:02:13 +0000 (UTC) Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by mail.notmuchmail.org (Postfix) with ESMTPS id EC4B55E012 for ; Wed, 12 Oct 2022 20:02:10 +0000 (UTC) Received: (Authenticated sender: robin@jarry.cc) by mail.gandi.net (Postfix) with ESMTPSA id 64359100002; Wed, 12 Oct 2022 20:02:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jarry.cc; s=gm1; t=1665604930; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=l4vhW0oZIy0UBBjp6rgJg+gvghYF6/O1uZBQh9G2scc=; b=R2VLT99Guofmh0jAW0UpIvnwz9HEyLVeActvz3+WKnbdTM7DtHrbx9HVzdi3csMQtmBEkX i4lNGG26GeLz2sDDxL9jojR76vn4CeOuvo2yPrpCXDEE5FA7VFr9ze5rs5C6AxwF0OS9Og gzaJ2sXbyeWjnoXcLhxPnM3brX4kp9L9qyInpr0hu7LQB8pIJnQ46EdMC0nroXK4gFPRqL EjBdBfivd8qVGxXnfm8zwEL4smMCe7H7utn+dk0TTVqnkyHFhUfJOT3V4e1W2/rTk7qWUO e+eLbdyBa00sw2jwVsCzf97Hl8gqRSq75bhV84KUMlyEtX6Z+nYZ8eJav3xLCA== Mime-Version: 1.0 Date: Wed, 12 Oct 2022 22:02:08 +0200 Message-Id: Subject: Re: [PATCH] cli: add options --offset and --limit to notmuch show From: "Robin Jarry" To: "Tomi Ollila" , X-Mailer: aerc/0.12.0-94-g001d45698f58 References: <20221011221942.341732-1-robin@jarry.cc> In-Reply-To: Message-ID-Hash: KRGXETQDZDLY5PYMSW6Q57EXBIGAVJ7H X-Message-ID-Hash: KRGXETQDZDLY5PYMSW6Q57EXBIGAVJ7H X-MailFrom: robin@jarry.cc X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-notmuch.notmuchmail.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Tim Culverhouse X-Mailman-Version: 3.3.3 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_IN X-Migadu-Country: DE ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1665604936; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-owner:list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=BTSBykq7sWxPnIaBMVsFf4LUxVQEAk8pu9eh36kUmaE=; b=OcmS9jFd4eu0NmgGukltuUKFtZcy7i7EGqdyfFeVJXbO7pBE5Dk72JI9Q8eS+iGjGzowF6 AMrNCVpT/BPrLY199vUMJaKrxBjQIZ97BS+aunDTQ+rjfi3K7e6IpyReh+7X1QCo0ekUik R9QJuONzbbWwc/I6uYjympeE8eYWNCa8iOavq6q2pYohNAstL8NfGNSZEOnJzut6dOJEv9 e3++KDY3cxi5kAzgnr65g4mIvRtOB1uuecaEg67DIMr+B17LiNJm15hBPr+dbNVMEVbNIB ZkR2gyAEcM29ArXeCffLMNEfqJon/2ZH0OVCLdftFRMUKOsS+97/jeGJWL1I1w== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1665604936; a=rsa-sha256; cv=none; b=kMOAALA4Hx3kK4piiZ97pc5S1ZdGy1uBvj3+NE4TnzEFvaiAj/D7KvYSCuS3zvqsCBOML8 4lZa1lQTeZD6cG/2AiNFIM8k5bHkjgQedD9K/UmtakLTMs1EFiMmW/8XELpdZ3oxGN+uk3 UGyjlic/zH0WV4ifTb0V1YjQsCtn+se2ll5r2+PMQBScqvMjMMGWXBNmqf8Xj5IHVXNIxy aY4fIObJOpznB+CPOWQum9KQvPhLGAgOwXs8CLSYU+h3mF/llhZdaWXIptLPYUFw2L2KVy pSs4jC5NNUw5rxUth3lWiyGKVQo7vAAAD16I/Ap8SNPDlR4yKG9Bal+RpYPGcA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=jarry.cc header.s=gm1 header.b=R2VLT99G; dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 135.181.149.255 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Spam-Score: 3.73 Authentication-Results: aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=jarry.cc header.s=gm1 header.b=R2VLT99G; dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 135.181.149.255 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Queue-Id: 46F1E3CBC6 X-Spam-Score: 3.73 X-Migadu-Scanner: scn1.migadu.com X-TUID: Qd7C/UKZMvGI Hi Tomi, Tomi Ollila, Oct 12, 2022 at 21:39: > > diff --git a/notmuch-show.c b/notmuch-show.c > > index ee9efa7448d7..ad31e0123268 100644 > > --- a/notmuch-show.c > > +++ b/notmuch-show.c > > @@ -1159,6 +1159,18 @@ do_show_threaded (void *ctx, > > notmuch_thread_t *thread; > > notmuch_messages_t *messages; > > notmuch_status_t status, res = NOTMUCH_STATUS_SUCCESS; > > + int i; > > + > > + if (params->offset < 0) { > > + unsigned count; > > + notmuch_status_t s = notmuch_query_count_threads (query, &count); > > + if (print_status_query ("notmuch search", query, s)) > > + return 1; > > + > > + params->offset += count; > > + if (params->offset < 0) > > this check and setting it to 0 is mystic to me, probably same code as in > search (?) probably it is good (?) (will not comment the same below) Yes, I copy/pasted that code from notmuch-search.c. I believe this to handle the case where: --limit=N && --offset=-M && M > count After adding count to offset, you may end up with a negative offset which makes the loop exit condition invalid and the --limit value would not be enforced as expected: i < params->offset + params->limit > > diff --git a/test/T131-show-limiting.sh b/test/T131-show-limiting.sh > > new file mode 100755 > > index 000000000000..a3da35944a3e > > --- /dev/null > > +++ b/test/T131-show-limiting.sh > > @@ -0,0 +1,81 @@ > > +#!/usr/bin/env bash > > +test_description='"notmuch show" --offset and --limit parameters' > > +. $(dirname "$0")/test-lib.sh || exit 1 > > + > > +add_email_corpus > > + > > +function show() { > > 'function' not used in other function defitions in other files, so this is > inconsistent (otherwise the content looks "better" than what I see usual ;D) I concur that this is a bash-only construct. I could remove the function keyword and we would have the same result. > > + test_begin_subtest "${outp}: offset = 0" > > inconsistent ${outp} (where $outp used elsewhere) ... Indeed. I had removed all the ${} except this one. For consistency with the other test scripts, I could add them back everywhere. I don't mind either way. I'll hold off for other remarks before sending a v2. Cheers,