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

[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?

> PS: I'm not sure the original code from
> before 14ff920dc885636a763d6ab7f256cc9981c24781 was correct either. It
> used to insert "x\n\n" (3 characters) on display (x being the
> propertized image) and removed via (delete-region b (+ b 2)) exactly 2
> characters. The new code after that revision inserted "x\n" (2
> characters, x being the propertized image) and removed via
> (delete-region b (1+ b)) exactly 1 character. So it might be possible
> that this off-by-one error in `mm-inline-image' has always existed.

Yes, I think so.