From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp11.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms12.migadu.com with LMTPS id kEEGAHZLhGQhqAAATFOONw (envelope-from ) for ; Sat, 10 Jun 2023 12:07:50 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp11.migadu.com with LMTPS id 8KccAHZLhGTwzwAA9RJhRA (envelope-from ) for ; Sat, 10 Jun 2023 12:07:50 +0200 Received: from mail1.fsfe.org (mail1.fsfe.org [217.69.89.151]) (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 D3BC93A46D for ; Sat, 10 Jun 2023 12:07:49 +0200 (CEST) From: Jelle Licht DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fsfe.org; s=2021100501; t=1686391668; 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=kvMFUMMd7fJIaSklD9OYJEJHn5jCRQtDtUgSIuaGWSU=; b=hcVba5VyUGkiAVlmm0mJTICVkeUJ9bHpfVba31ln77JlETJuIfwv1IWJN76fO+JNI9MWU9 Ni8GUrqRfIUG7sCO3TJRM+C4wOYNqpJpAH9+KS4oUT+qkQUVUedIOOFLrE7zyH4RV82NNa T0Ajj3YT+hApd0wDPx9xfuS8YqN5Kvk= To: Kyle Meyer Cc: piem@inbox.kyleam.com Subject: Re: [PATCH v2 2/5] gnus: Skip adding mboxrd from-line when not needed. In-Reply-To: <87ttvfncfm.fsf@kyleam.com> References: <87ttvfncfm.fsf@kyleam.com> Date: Sat, 10 Jun 2023 12:07:48 +0200 Message-ID: <87jzwbmxzf.fsf@fsfe.org> MIME-Version: 1.0 Content-Type: text/plain X-Migadu-Country: DE X-Migadu-Flow: FLOW_IN Authentication-Results: aspmx1.migadu.com; none X-Migadu-Scanner: scn0.migadu.com X-Migadu-Spam-Score: -4.00 X-Spam-Score: -4.00 X-Migadu-Queue-Id: D3BC93A46D X-TUID: /+bXrNpZ5toU Kyle Meyer writes: > jlicht@fsfe.org writes: > >> From: Jelle Licht > > Could you add a bit about why this is needed? Basically that, at least > over NNTP, gnus-summary-display-article shows a plain message, but when > debbugs.el visits an issues message with Gnus, it's already in mbox > format. I copied over this explanation into the commit message for V3. > > It'd also be nice to add a link to the relevant inbox.kyleam.com thread. Done in V3. > (Convention nit-pick for this and subsequent patches: please drop the > period from the subject, ignoring all Guix muscle memory :>) Big ask, but done in V3 ;-). >> +(defun piem-gnus--from-line (buffer) >> + "Returns a cons of the first line of BUFFER, if it is an >> +mbboxrd from-line (or nil if none), and the remaining lines of > > typo: s/mbboxrd/mboxrd/ Done in V3. >> +BUFFER." > > To follow Elisp docstring conventions, could you rewrite this so that > the first line is self-contained sentence? My best attempt at this, done in V3. > It's marked as internal, so not too important, but I wonder if something > like "piem-gnus--split-message" might give a better hint at what it > does. I like the current name, but not enough get out my soapbox. Concretely, feel free to change it if you feel strongly when applying the patches; otherwise, leaving it as-is in V3 :-). >> + (with-current-buffer buffer >> + (let ((start (point-min)) >> + (end (point-max))) >> + (goto-char start) >> + (let* ((eol (line-end-position)) >> + (line (buffer-substring-no-properties start eol))) >> + (if (string-match-p "^From " line) >> + (cons line (buffer-substring-no-properties (+ eol 1) end)) >> + (cons nil (buffer-substring-no-properties start end))))))) > > Looks fine to me. And I don't think we need to worry about > save-excursion here, given that we're triggering the display of these > buffers and, in general, this command is making lots of user-visible > changes to the layout anyway (and we didn't know a way to avoid that: > <87turabsb0.fsf@yoctocell.xyz>). I looked into save-excursion here as well, but as you mention things are already making several visible changes. A 'gnus-save-excursion' macro would be nifty, but seems out-of-scope for piem imho.