all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Alcor via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 69920@debbugs.gnu.org
Subject: bug#69920: Proposed fix - Toggling MIME inline attachment previews adds superfluous newlines
Date: Sat, 23 Mar 2024 10:53:22 +0100	[thread overview]
Message-ID: <874jcxmfzh.fsf@tilde.club> (raw)
In-Reply-To: <86sf0hzb5c.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 23 Mar 2024 08:59:27 +0200")

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

If it helps clarify things, I'm okay with renaming the bug report to
something like "MIME inline image preview undisplayer does not clean up
displayed image correctly" or something along these lines. When I filed
the report, I wasn't sure about the cause so I described the user-facing
issue instead of the actual technical problem.

Cheers,
-A.

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.





  reply	other threads:[~2024-03-23  9:53 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 [this message]
2024-03-23 10:20         ` Eli Zaretskii
2024-04-06  8:59           ` Eli Zaretskii
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=874jcxmfzh.fsf@tilde.club \
    --to=bug-gnu-emacs@gnu.org \
    --cc=69920@debbugs.gnu.org \
    --cc=alcor@tilde.club \
    --cc=eliz@gnu.org \
    /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.