[PATCH] Unindentation fixup for code blocks

[PATCH] Unindentation fixup for code blocks

[Psionic K]

When cleaning up hard indentation, I found my source blocks remaining indented.

The way that org-do-remove-indentation is sensitive to invisible text.
This occurs for source blocks whenever using org-modern-mode, where it
makes the source blocks align left.

By swapping out the arithmetic and expressions that relied on
current-column, the behavior is fixed.

There are several behaviors I only just found, so I don't expect a lot
of precision in my fix, but the patch I came up with follows:

From 858077f0d2a7f4cd8699948229c2965f0c6bb0a1 Mon Sep 17 00:00:00 2001
From: Psionik K <73710933+psionic-k@users.noreply.github.com>
Date: Wed, 10 Jan 2024 18:05:53 +0900
Subject: [PATCH] when removing indentation, take into account invisible text

org-current-text-indentation sets invisibility spec to nil
temporarily.

One behavioral difference is that the point is no longer moved
forward.  Therefore, arithmetic is used instead of (current-column)
based math.  (current-column) is inaccurate when moving the point
through invisible text.
---
 lisp/org-macs.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 8def5cbb..0512cc48 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -408,13 +408,13 @@ line.  Return nil if it fails."
     ;; Remove exactly N indentation, but give up if not possible.
         (when skip-fl (forward-line))
     (while (not (eobp))
-      (let ((ind (progn (skip-chars-forward " \t") (current-column))))
+      (let ((ind (org-current-text-indentation)))
         (cond ((< ind n)
-                   (if (eolp) (delete-region (line-beginning-position) (point))
+                   (if (eolp) (delete-region (line-beginning-position)
+                                             (line-end-position))
                      (throw :exit nil)))
           (t (delete-region (line-beginning-position)
-                                    (progn (move-to-column n t)
-                                           (point)))))
+                                    (+ (line-beginning-position) n))))
         (forward-line)))
     ;; Signal success.
     t))))
-- 
2.42.0

Re: [PATCH] Unindentation fixup for code blocks

[Ihor Radchenko]

Psionic K <psionik@positron.solutions> writes:

> When cleaning up hard indentation, I found my source blocks remaining indented.
>
> The way that org-do-remove-indentation is sensitive to invisible text.
> This occurs for source blocks whenever using org-modern-mode, where it
> makes the source blocks align left.

Thanks for the patch!
Would you also be able to create a reproducer, so that we can replicate
the problem and write a test?

> By swapping out the arithmetic and expressions that relied on
> current-column, the behavior is fixed.
>
> There are several behaviors I only just found, so I don't expect a lot
> of precision in my fix, but the patch I came up with follows:
>
> From 858077f0d2a7f4cd8699948229c2965f0c6bb0a1 Mon Sep 17 00:00:00 2001
> From: Psionik K <73710933+psionic-k@users.noreply.github.com>
> Date: Wed, 10 Jan 2024 18:05:53 +0900
> Subject: [PATCH] when removing indentation, take into account invisible text

It looks like you did not send the patch with git send-email (this email
patch does not apply with git). It might be easier if you simply attach
the patch to email.

> One behavioral difference is that the point is no longer moved
> forward.  Therefore, arithmetic is used instead of (current-column)
> based math.  (current-column) is inaccurate when moving the point
> through invisible text.

If you can, please add changelog entries to the commit message. See
https://orgmode.org/worg/org-contribute.html#org77afce2

> -                                    (progn (move-to-column n t)
> -                                           (point)))))
> +                                    (+ (line-beginning-position) n))))

This math is not accurate when tabs are present.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Unindentation fixup for code blocks

[Psionic K]

[-- Attachment #1: Type: text/plain, Size: 785 bytes --]

> Would you also be able to create a reproducer, so that we can replicate
the problem and write a test?

Yeah, I just have to make some indentation text invisible.  Where
should such a test live?  Where is your documentation on running a
test?

> It looks like you did not send the patch with git send-emai.
Nope.  Magit patch create.  Attaching another patch.

> This math is not accurate when tabs are present.

You want to short circuit this kind of conversation.  I was not
actually aware of any tabs versus spaces aware functions to build on
top of since I always use spaces.  Had I not already found another way
to correct the behavior, this would have been a dead-end I would have
had to round trip.

I'm presuming tests don't get their own changelog entries.  Is that correct?

[-- Attachment #2: 0001-org-macs.el-org-do-remove-indentation-correction.patch --]
[-- Type: text/x-patch, Size: 1224 bytes --]

From 965b60f58512b009778b7b5279394bf01d407c3f Mon Sep 17 00:00:00 2001
From: Psionik K <73710933+psionic-k@users.noreply.github.com>
Date: Wed, 10 Jan 2024 22:37:28 +0900
Subject: [PATCH] org-macs.el: org-do-remove-indentation correction

* lisp/org-macs.el (org-do-remove-indentation): set
`buffer-invisibility-spec' to nil before detecting the column or
moving to a column.

This fixes src_block indentation removal for org-modern-mode but will
also correct other cases of hidden indentation.
---
 lisp/org-macs.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 8def5cbb..7f5d2ad8 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -408,7 +408,8 @@ line.  Return nil if it fails."
 	;; Remove exactly N indentation, but give up if not possible.
         (when skip-fl (forward-line))
 	(while (not (eobp))
-	  (let ((ind (progn (skip-chars-forward " \t") (current-column))))
+	  (let* ((buffer-invisibility-spec nil)
+                 (ind (progn (skip-chars-forward " \t") (current-column))))
 	    (cond ((< ind n)
                    (if (eolp) (delete-region (line-beginning-position) (point))
                      (throw :exit nil)))
-- 
2.42.0

Re: [PATCH] Unindentation fixup for code blocks

[Ihor Radchenko]

Psionic K <psionik@positron.solutions> writes:

>> Would you also be able to create a reproducer, so that we can replicate
> the problem and write a test?
>
> Yeah, I just have to make some indentation text invisible.  Where
> should such a test live?  Where is your documentation on running a
> test?

The tests live in tests/lisp folder.
See https://git.savannah.gnu.org/cgit/emacs/org-mode.git/tree/testing/README

>> It looks like you did not send the patch with git send-emai.
> Nope.  Magit patch create.  Attaching another patch.

Thanks! This patch does apply.
I am not yet sure if I like this patch or the previous one more. I am
thinking about combining them and implementing something like
`org-move-to-text-column'.

>> This math is not accurate when tabs are present.
>
> You want to short circuit this kind of conversation.  I was not
> actually aware of any tabs versus spaces aware functions to build on
> top of since I always use spaces.  Had I not already found another way
> to correct the behavior, this would have been a dead-end I would have
> had to round trip.

I am not sure what you mean.

> I'm presuming tests don't get their own changelog entries.  Is that correct?

Tests are a part of Org codebase. They do have their own changelog
entries inside commit messages that introduce such tests. For example,
see https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=d0d838b02e44a40adca14d6eae39fd4c364730da

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Unindentation fixup for code blocks

[Ihor Radchenko]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

Ihor Radchenko <yantar92@posteo.net> writes:

> Thanks! This patch does apply.
> I am not yet sure if I like this patch or the previous one more. I am
> thinking about combining them and implementing something like
> `org-move-to-text-column'.

I do not see immediate use of `org-move-to-text-column', so let's keep
things simple.

I am attaching an amended patch with modified commit message and
docstring describing the invisible text is accounted as visible.
I also added TINYCHANGE cookie as you do not appear to have FSF
copyright assignment.

Let me know if you want to make any changes to the new version of the
patch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-org-do-remove-indentation-Ignore-invisible-text.patch --]
[-- Type: text/x-patch, Size: 1815 bytes --]

From b7eb76647395eac256602bdecaab4aea3ffbe68c Mon Sep 17 00:00:00 2001
Message-ID: <b7eb76647395eac256602bdecaab4aea3ffbe68c.1705838536.git.yantar92@posteo.net>
From: Psionik K <73710933+psionic-k@users.noreply.github.com>
Date: Wed, 10 Jan 2024 22:37:28 +0900
Subject: [PATCH v2] org-do-remove-indentation: Ignore invisible text

* lisp/org-macs.el (org-do-remove-indentation): Set
`buffer-invisibility-spec' to nil before detecting the column or
moving to a column.

This fixes src_block indentation removal for org-modern-mode but will
also correct other cases of hidden indentation.

TINYCHANGE
---
 lisp/org-macs.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 8def5cbb2..fc4fd7975 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -388,6 +388,8 @@ (defmacro org-current-text-indentation ()
 
 (defun org-do-remove-indentation (&optional n skip-fl)
   "Remove the maximum common indentation from the buffer.
+Do not consider invisible text when calculating indentation.
+
 When optional argument N is a positive integer, remove exactly
 that much characters from indentation, if possible.  When
 optional argument SKIP-FL is non-nil, skip the first
@@ -408,7 +410,8 @@ (defun org-do-remove-indentation (&optional n skip-fl)
 	;; Remove exactly N indentation, but give up if not possible.
         (when skip-fl (forward-line))
 	(while (not (eobp))
-	  (let ((ind (progn (skip-chars-forward " \t") (current-column))))
+	  (let* ((buffer-invisibility-spec nil) ; do not treat invisible text specially
+                 (ind (progn (skip-chars-forward " \t") (current-column))))
 	    (cond ((< ind n)
                    (if (eolp) (delete-region (line-beginning-position) (point))
                      (throw :exit nil)))
-- 
2.43.0


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Unindentation fixup for code blocks

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> I am attaching an amended patch with modified commit message and
> docstring describing the invisible text is accounted as visible.
> I also added TINYCHANGE cookie as you do not appear to have FSF
> copyright assignment.

Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b6f8078ab

You are also now listed as Org mode contributor.
https://git.sr.ht/~bzg/worg/commit/beb9318d

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>