GNU bug report logs - #68185
[PATCH] Ensure indent-region argument order is correct

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 68185 in the body.
You can then email your comments to 68185 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Morgan Willcock <morgan <at> ice9.digital>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Ensure indent-region argument order is correct
Date: Sun, 31 Dec 2023 21:20:11 +0000

[Message part 1 (text/plain, inline)]

Tags: patch

The attached patch fixes an issue with Tempo template insertion, where
the region start is assumed to be a position before the region end.  If
the user selects a region in a backwards direction the correct
indentation will not be applied:

  (require 'tempo)
  (tempo-define-template "indent-region-test" '(r>))

  (with-temp-buffer
    (emacs-lisp-mode)
    (insert "    one\n    two\n    three")
    (goto-char (point-min))
    (set-mark-command nil)
    (goto-char (point-max))
    (tempo-template-indent-region-test)
    (buffer-string))

  ;; => "one\ntwo\nthree"

  (with-temp-buffer
    (emacs-lisp-mode)
    (insert "    one\n    two\n    three")
    (goto-char (point-max))
    (set-mark-command nil)
    (goto-char (point-min))
    (tempo-template-indent-region-test)
    (buffer-string))

  ;; => "    one\n    two\nthree"

The start and end positions are already identified and stored using
separate variables, so the patch just uses these values instead of
assuming the point and mark order.  I've also removed passing nil as the
third argument since the argument is optional.

In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.16.0, Xaw3d scroll bars) of 2023-07-31 built on inspiron
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Debian GNU/Linux 12 (bookworm)

Configured using:
 'configure --with-native-compilation --with-cairo --with-json
 --with-xml2 --with-x-toolkit=lucid'

[0001-Ensure-indent-region-argument-order-is-correct.patch (text/patch, attachment)]

Message #8 received at 68185 <at> debbugs.gnu.org (full text, mbox):

From: Morgan Willcock <morgan <at> ice9.digital>
To: 68185 <at> debbugs.gnu.org
Subject: [PATCH] Ensure indent-region argument order is correct
Date: Sun, 31 Dec 2023 22:39:33 +0000

Apologies, I think the fix is too simplistic and does not account for
the region being shifted.  The existing implementation does account for
that for also assumes that the region was selected in a forwards
direction.

Is it possible to convert this into a normal bug report rather than a
patch request?

Message #11 received at 68185 <at> debbugs.gnu.org (full text, mbox):

From: Morgan Willcock <morgan <at> ice9.digital>
To: 68185 <at> debbugs.gnu.org
Subject: Re: [PATCH] Ensure indent-region argument order is correct
Date: Mon, 01 Jan 2024 17:04:30 +0000

[Message part 1 (text/plain, inline)]

Attached is a replacement patch which I believe fixes the issue with the
region selection direction and with indentation potentially being
incorrectly applied to text that is inserted by the template.

Previously when text was being inserted at the region start it was
actually going into region, which means that it will have been possible
for template contents to have accidentally been indented along with the
region that the user selected.  I've changed the marker insertion type
for the start boundary, which should preserve the original indentation
of text which was inserted by the template, i.e. if text inserted by the
template should be indented this is now back under control of the
template rather than applied by accident.

The changes do mean that the indentation of existing templates may
change, but I think this will only be for templates which accidentally
omitted the indentation directives because indentation was mistakenly
applied as part of the region.

[0001-Ensure-indent-region-arguments-and-boundaries-are-co.patch (text/x-diff, attachment)]

Message #16 received at 68185-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Morgan Willcock <morgan <at> ice9.digital>
Cc: 68185-done <at> debbugs.gnu.org
Subject: Re: bug#68185: [PATCH] Ensure indent-region argument order is correct
Date: Thu, 04 Jan 2024 13:18:16 +0200

> From: Morgan Willcock <morgan <at> ice9.digital>
> Date: Sun, 31 Dec 2023 21:20:11 +0000
> 
> The attached patch fixes an issue with Tempo template insertion, where
> the region start is assumed to be a position before the region end.  If
> the user selects a region in a backwards direction the correct
> indentation will not be applied:

Thanks, installed on the master branch, and closing the bug.

Message #19 received at 68185 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Morgan Willcock <morgan <at> ice9.digital>
Cc: 68185 <at> debbugs.gnu.org
Subject: Re: bug#68185: [PATCH] Ensure indent-region argument order is correct
Date: Thu, 04 Jan 2024 13:22:56 +0200

> From: Morgan Willcock <morgan <at> ice9.digital>
> Date: Mon, 01 Jan 2024 17:04:30 +0000
> 
> Attached is a replacement patch which I believe fixes the issue with the
> region selection direction and with indentation potentially being
> incorrectly applied to text that is inserted by the template.
> 
> Previously when text was being inserted at the region start it was
> actually going into region, which means that it will have been possible
> for template contents to have accidentally been indented along with the
> region that the user selected.  I've changed the marker insertion type
> for the start boundary, which should preserve the original indentation
> of text which was inserted by the template, i.e. if text inserted by the
> template should be indented this is now back under control of the
> template rather than applied by accident.
> 
> The changes do mean that the indentation of existing templates may
> change, but I think this will only be for templates which accidentally
> omitted the indentation directives because indentation was mistakenly
> applied as part of the region.

Thanks, installed (the additional part which wasn't in the previous
patch).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.