From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp12.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id CD2hBdYaSWPP7gAAbAwnHQ (envelope-from ) for ; Fri, 14 Oct 2022 10:16:22 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp12.migadu.com with LMTPS id yMuEBdYaSWOrdQEAauVa8A (envelope-from ) for ; Fri, 14 Oct 2022 10:16:22 +0200 Received: from mail.notmuchmail.org (yantan.tethera.net [IPv6:2a01:4f9:c011:7a79::1]) (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 D482C3C93F for ; Fri, 14 Oct 2022 10:16:21 +0200 (CEST) Received: from yantan.tethera.net (localhost [127.0.0.1]) by mail.notmuchmail.org (Postfix) with ESMTP id E30115F841; Fri, 14 Oct 2022 08:16:18 +0000 (UTC) Received: from meesny.iki.fi (meesny.iki.fi [195.140.195.201]) by mail.notmuchmail.org (Postfix) with ESMTPS id CAAAD5F361 for ; Fri, 14 Oct 2022 08:16:15 +0000 (UTC) Received: from c53 (gw1.nor.fi [185.218.193.67]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: too) by meesny.iki.fi (Postfix) with ESMTPSA id 16D79200C9; Fri, 14 Oct 2022 11:16:15 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1665735375; 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: in-reply-to:in-reply-to:references:references; bh=zXrZaY0ACtCtkHwcpjTCiTB2YlufQ8ktdfcnVaM9EBs=; b=Lx2pqMQJn8xVpQ6xO5lydm+BOS8QWDfMja3y4zHjTHP9sntCpiKzC/nwz6fu/VQHbso07m vfsy4b+J2xczgIxxFNofDdaG+AUnlObZnTQSq7dwcIg2FcBehA39+44lzQYwesSVmkZA8G /13K2N3PUa++TNtsciqzXsmBQGSxOuA= From: Tomi Ollila To: Robin Jarry , notmuch@notmuchmail.org Subject: Re: [PATCH] cli: add options --offset and --limit to notmuch show In-Reply-To: References: <20221011221942.341732-1-robin@jarry.cc> User-Agent: Notmuch/0.37+18~g2a896dd (https://notmuchmail.org) Emacs/27.1 X-Face: HhBM'cA~ MIME-Version: 1.0 ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=too smtp.mailfrom=tomi.ollila@iki.fi ARC-Seal: i=1; s=meesny; d=iki.fi; t=1665735375; a=rsa-sha256; cv=none; b=a/XCUEOJXcPHgs9yRzGy0VVAooYcEcf6R2xIcyBSsA/ksaKI3KLWXKoTf/DENTTOKXEPbT c+GF9PLyc8sMVtIGF6yOnsrQu/CQb5g0G1XtnGcZDHcW4uXjf2jqhYm2h/BbPjUGno7VaR d5yRUFlNydwJBt4kYi3z1Bi5g5VTdyQ= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=meesny; t=1665735375; 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: in-reply-to:in-reply-to:references:references; bh=zXrZaY0ACtCtkHwcpjTCiTB2YlufQ8ktdfcnVaM9EBs=; b=FFfayTR7GhFfHOpTfAXxbmawZTzJB+SmL59lkYAVDRunkhHTIWyz43zOIbBpGIwsCKgTqQ SNdUuzjBUJ5gAy2NF5gJRMdYQEJWeNAElneqJskPD4WYkyfxOAAfqRu2F34oiyHiOgeXq2 QILakZNvsnLup2pVJ4TfomKnGdycvxw= Message-ID-Hash: DYD446NYCEEDGYG4IKWCI2VDGSZ6ZTWQ X-Message-ID-Hash: DYD446NYCEEDGYG4IKWCI2VDGSZ6ZTWQ X-MailFrom: tomi.ollila@iki.fi 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=2; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1665735382; 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=uumzUegbyYsOzhWzTJY+igdjAesVQl1g+X6woE3b3Wo=; b=S/QNOIBxh7FuX7msfpH5ipZzisPO9mDzPutltDN3lQqXBY+f0rulJibOcCCQPF9KORnn1F ARFl42WSnfe1wgSChf7wqLrbtXp3RD4FaAhPrACmURTHvuXxOsBFArkjtkmfVI4BFJxJmF sX8fUyI9nCpZv24YDukvEIAG2P1JrEggP1INsuOFAsmSEF0Xo2JJ2si/jQbyRts7iDM+ml hpVK81ncgyHXFu5ckqmETNWQF11tV/HPEkLmCnPRl7asc8nbYJW6spRA2LQ7tEHH08N22a SHWCV3dfYeryOX29R6nDFgNWol6FRcc5LsMRtcdNrQrS7u6UoYBLErguAq9gbg== ARC-Seal: i=2; s=key1; d=yhetil.org; t=1665735382; a=rsa-sha256; cv=fail; b=tDthU2RdV1hwpQKcuKIcriKEgL0qmd0s+6sjYfgZQp9DCj5w22+Vd8CROb/ixEltEXEINr JVUTdwEGQMQDCsj28y1IZ/kH3sHCUaWIxX4Z2d6Jjy1oMJHDR7dFLS/3tgbUhJ16cebezf 7I5y9v9b3dxYsYExapeufljOVJO0j1zQWpd1h8d4qhPPVQ1nCfvtOu9nOvBKjh+bsel8bE XO5WbEsc7pIDU0JPuIX2onuIMKlPnIHj6g3NVqgTiAQe76Gvx9HyGj4E+XJjnS9Epgta2n ka98tB/5jAOGiDdyOPyUmfmLUcOSkBDgUYmBpncp9tnVfvqn8K6VTs1JEVt0HA== ARC-Authentication-Results: i=2; aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=iki.fi header.s=meesny header.b=Lx2pqMQJ; arc=reject ("signature check failed: fail, {[1] = sig:iki.fi:reject}"); dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 2a01:4f9:c011:7a79::1 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Spam-Score: 3.91 Authentication-Results: aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=iki.fi header.s=meesny header.b=Lx2pqMQJ; arc=reject ("signature check failed: fail, {[1] = sig:iki.fi:reject}"); dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 2a01:4f9:c011:7a79::1 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Queue-Id: D482C3C93F X-Spam-Score: 3.91 X-Migadu-Scanner: scn1.migadu.com X-TUID: AjtMMDK0G+Cp On Wed, Oct 12 2022, Robin Jarry wrote: > 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. Bash constructs are allowed (even perhaps encouraged in many cases), I just care about consistency (it is easier to browse through large set of files if there is not too much differenct constructs used) In case of 'function' it also works in zsh (and probably ksh). I have started to use in all (except one, I just noticed) function definitions in my zshrc. I recall I had a problem which using function fixed -- probably related to something with ff () { ...; } alias ff='noglob ff' which did not exactly worked like it should have. function ff () { ...; } fixed that case (cannot remember exact details, changed 2021-03-08 w/ somewhat vague git commit message (in private dotdir repo :))) > >> > + 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 personally prefer w/o {} in trivial cases, shorter and clearer to read (and use $FOO''bar instead of ${FOO}bar when such need arrives ;/). that is such a small difference (IMO) that either goes... > > I'll hold off for other remarks before sending a v2. > > Cheers, Tomi