Re: [PATCH v2 2/5] gnus: Skip adding mboxrd from-line when not needed.

[Jelle Licht <jlicht@fsfe.org>]

Kyle Meyer <kyle@kyleam.com> writes:

> jlicht@fsfe.org writes:
>
>> From: Jelle Licht <jlicht@fsfe.org>
>
> 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.