all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: eric@ericabrahamsen.net
Cc: 69920@debbugs.gnu.org, alcor@tilde.club
Subject: bug#69920: Proposed fix - Toggling MIME inline attachment previews adds superfluous newlines
Date: Sat, 06 Apr 2024 11:59:33 +0300	[thread overview]
Message-ID: <864jce50ju.fsf@gnu.org> (raw)
In-Reply-To: <86bk75z1un.fsf@gnu.org> (message from Eli Zaretskii on Sat, 23 Mar 2024 12:20:16 +0200)

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





  reply	other threads:[~2024-04-06  8:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-20 17:59 bug#69920: 29.2; gnus: article-mode: Toggling MIME inline attachment previews adds superfluous newlines Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-22 21:01 ` bug#69920: Proposed fix - " Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-22 22:25   ` Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-23  6:59     ` Eli Zaretskii
2024-03-23  9:53       ` Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-23 10:20         ` Eli Zaretskii
2024-04-06  8:59           ` Eli Zaretskii [this message]
2024-04-18  9:00             ` Eli Zaretskii
2024-04-22  2:48               ` Eric Abrahamsen
2024-04-22 12:42                 ` Alcor via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-22 14:58                   ` bug#69920: 29.2; gnus: article-mode: " Eric Abrahamsen
2024-03-23  6:52   ` bug#69920: Proposed fix - " Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=864jce50ju.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=69920@debbugs.gnu.org \
    --cc=alcor@tilde.club \
    --cc=eric@ericabrahamsen.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.