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: Thu, 18 Apr 2024 12:00:21 +0300	[thread overview]
Message-ID: <86bk67f3kq.fsf@gnu.org> (raw)
In-Reply-To: <864jce50ju.fsf@gnu.org> (message from Eli Zaretskii on Sat, 06 Apr 2024 11:59:33 +0300)

Ping! Ping! Eric, please chime in.

> 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?
> > 
> > > 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-18  9:00 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
2024-04-18  9:00             ` Eli Zaretskii [this message]
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=86bk67f3kq.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.