bug#69920: Proposed fix - Toggling MIME inline attachment previews adds superfluous newlines

[Eric Abrahamsen <eric@ericabrahamsen.net>]

On 04/18/24 12:00 PM, Eli Zaretskii wrote:
> Ping! Ping! Eric, please chime in.

Sorry this has taken so long!

>> Cc: 69920@debbugs.gnu.org, alcor@tilde.club
>> Date: Sat, 06 Apr 2024 11:59:33 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> Ping! Eric, can you please chime in?
>> 
>> > Cc: 69920@debbugs.gnu.org
>> > Date: Sat, 23 Mar 2024 12:20:16 +0200
>> > From: Eli Zaretskii <eliz@gnu.org>
>> > 
>> > > From: Alcor <alcor@tilde.club>
>> > > Cc: 69920@debbugs.gnu.org
>> > > Date: Sat, 23 Mar 2024 10:53:22 +0100
>> > > 
>> > > Eli Zaretskii <eliz@gnu.org> writes:
>> > > 
>> > > > Are you saying that the problem is with the function that
>> > > > "un-displays" the inline image, in that it fails to remove the
>> > > > inserted newline?  (AFAIU, the code before the above commit also had
>> > > > the same issue.)  That wasn't clear from the description of the
>> > > > problem, and the Subject is ambiguous wrt what newlines are deemed
>> > > > "superfluous".  So please clarify what is the problem you are flagging
>> > > > here.
>> > > 
>> > > The problem is that the undisplayer does not correctly undo what the
>> > > displayer does.
>> > > 
>> > > The undisplayer (as it is right now) just removes the image. That can be
>> > > confirmed by stepping through the code with edebug or by inspecting the
>> > > "b" variable.
>> > > 
>> > > I think we can agree that (delete-region b (1+ b)) will always delete
>> > > _exactly one_ character, and in that case that would be the propertized
>> > > "x" with the image. That would leave a dangling newline.
>> > > 
>> > > There are two ways to solve this:
>> > > 
>> > > 1. Do not add the extra newline (this is what the patch does).
>> > > 2. Remove the extra newline via (delete-region b (+ b 2)) – note that I
>> > > have not tried this, but it would make sense to me.
>> > > 
>> > > I happen to prefer option #1 as the extra newline does not seem to have
>> > > any meaningful function. But this is just my own preference (Emacs/gnus
>> > > maintainers may wish to retain the extra newline if it serves a valid purpose).
>> > 
>> > I prefer #2.  Eric, WDYT?

I also prefer #2, mostly because the other `mm-inline-*' functions here
all ensure that inlined content ends with a newline, and even if there's
no immediate consequence to not having it, I'd prefer the consistency.
Alcor, would you be willing to update your patch?

Thanks,
Eric