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

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