Re: [BUG] Source block indentation does not work properly for yaml-mode [9.6.6 ( @ /home/user/.emacs.d/elpa/org-9.6.6/)]

["Sébastien Miquel" <sebastien.miquel@posteo.eu>]

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


Ihor Radchenko writes:
>> +         ;; Trim contents: `org-src--contents-for-write-back' may have
>> +         ;; added indentation at the beginning, which we remove.
>
> May you also mention that we remove the indentation to avoid adding
> spaces to latex fragments in the middle of a paragraph?

On second thought, I don't think moving the LaTeX fragment logic away
from `org-src--contents-for-write-back` makes sense. This part of the
function does the opposite of `org-do-remove-indentation`, and the
latter has a boolean argument `skip-fl`, so it makes sense to keep it
the same here. It is simple enough.

If you're worried about consistency with inline src blocks, I find it
weird for them to have newlines, let alone newlines mixed with org's
indentation. But if we do want to treat them the same, then we also
need to modify `org-do-remove-indentation` to skip the first line for
them as well.

I've taken this part off the patch for now.

> If source code in the edit buffer contains non-empty blank lines, it is
> not Org's responsibility to clear them. In fact, it will go against
> possible user settings!
>
> So, I agree that we should not indent empty lines. However, I do not
> agree that we should not indent non-empty blank lines.

The patch I propose does indent non-empty blank lines. The issue is
with =org-do-remove-indentation= which empties blank lines. I've added
a fix to this.

>> -        (setq org-src--preserve-blank-line preserve-blank-line)
>> +        (setq org-src--indent-current-empty-line (and blank-line
>> +                                                      (not empty-line)))
>
> Here, you have a variable named "empty-line" set when (not empty-line). ??

I've renamed it yet again!

>
>>       (while (not (eobp))
>> -	  (skip-chars-forward " \t")
>> -          (when (or (not (eolp))                               ; not a blank line
>> -                    (and (eq (point) (marker-position marker)) ; current line
>> +          (when (or (not (eolp)) ; not an empty line
>> +                    ;; If the current line is empty, we may
>> +                    ;; want to indent it.
>> +                    (and (eq (point) (marker-position marker))
>>                           preserve-blank-line))
>>            (insert indent-str))
>> 	  (forward-line)))
>
> removed `skip-chars-forward' call, so the loop will always check every
> bol and (not (eolp)) will be t for every line, except ^$.
> Then, considering that preserve-blank-line is set when (not empty-line),
> your second condition will never trigger.
>
> I feel that something is fishy in the logic.

What happens is: in the org buffer, the line is not empty, because it
has the org indentation (which was possibly just added by
org-indent-line), but in the edit buffer, the line is empty, because
the common org indentation was removed. In that case, we want to add
back the org indentation.

--
Sébastien Miquel

[-- Attachment #2: 0001-org-src.el-Use-native-value-of-indent-tabs-mode-for-.patch --]
[-- Type: text/x-patch, Size: 9561 bytes --]

From ecc87b4a75dec7ff8fba4c86635ba3cdb43444ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= <sebastien.miquel@posteo.eu>
Date: Tue, 27 Jun 2023 09:23:01 +0200
Subject: [PATCH] org-src.el: Use native value of `indent-tabs-mode' for
 indentation

* lisp/org-macs.el (org-do-remove-indentation): Preserve
indentation (spaces vs tabs) past the common indentation to remove.
Do not empty blank lines.
* lisp/org-src.el (org-src--contents-for-write-back): Preserve the
native indentation (spaces vs tabs).  If necessary, add a common org
indentation to the block according to org's `indent-tabs-mode'.
(org-src-font-lock-fontify-block): In case of mixed indentation,
display the tab characters with a fixed width, according to the native
tab width value.
* testing/lisp/test-org-src.el (test-org-src/indented-blocks): Update
tests.  Indentation no longer obeys `indent-tabs-mode' from the org
buffer, but is separated in two parts.
---
 lisp/org-macs.el             |  9 +++--
 lisp/org-src.el              | 45 ++++++++++++++++------
 testing/lisp/test-org-src.el | 75 ++++++++++++++++++++++--------------
 3 files changed, 85 insertions(+), 44 deletions(-)

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 51dbfe118..ea210dc60 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -483,9 +483,12 @@ line.  Return nil if it fails."
         (when skip-fl (forward-line))
 	(while (not (eobp))
 	  (let ((ind (progn (skip-chars-forward " \t") (current-column))))
-	    (cond ((eolp) (delete-region (line-beginning-position) (point)))
-		  ((< ind n) (throw :exit nil))
-		  (t (indent-line-to (- ind n))))
+	    (cond ((< ind n)
+                   (if (eolp) (delete-region (line-beginning-position) (point))
+                     (throw :exit nil)))
+		  (t (delete-region (line-beginning-position)
+                                    (progn (move-to-column n t)
+                                           (point)))))
 	    (forward-line)))
 	;; Signal success.
 	t))))
diff --git a/lisp/org-src.el b/lisp/org-src.el
index f15ba8e99..9e6031016 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -474,11 +474,15 @@ Assume point is in the corresponding edit buffer."
                            (buffer-substring eol (point-max))))))
 	(write-back org-src--allow-write-back)
         (preserve-blank-line org-src--preserve-blank-line)
-        marker)
+        marker indent-str)
+    (setq indent-str
+          (with-temp-buffer
+            ;; Reproduce indentation parameters from org buffer.
+            (setq indent-tabs-mode use-tabs?)
+            (when (> source-tab-width 0) (setq tab-width source-tab-width))
+            (indent-to indentation-offset)
+            (buffer-string)))
     (with-current-buffer write-back-buf
-      ;; Reproduce indentation parameters from source buffer.
-      (setq indent-tabs-mode use-tabs?)
-      (when (> source-tab-width 0) (setq tab-width source-tab-width))
       ;; Apply WRITE-BACK function on edit buffer contents.
       (insert (org-no-properties (car contents)))
       (setq marker (point-marker))
@@ -488,15 +492,16 @@ Assume point is in the corresponding edit buffer."
       ;; Add INDENTATION-OFFSET to every line in buffer,
       ;; unless indentation is meant to be preserved.
       (when (> indentation-offset 0)
-	(when preserve-fl (forward-line))
+        ;; LaTeX-fragments are inline. Do not add indentation to their
+        ;; first line.
+        (when preserve-fl (forward-line))
         (while (not (eobp))
-	  (skip-chars-forward " \t")
-          (when (or (not (eolp))                               ; not a blank line
-                    (and (eq (point) (marker-position marker)) ; current line
+          (when (or (not (eolp)) ; not an empty line
+                    ;; If the current line is empty, we may
+                    ;; want to indent it.
+                    (and (eq (point) (marker-position marker))
                          preserve-blank-line))
-	    (let ((i (current-column)))
-	      (delete-region (line-beginning-position) (point))
-	      (indent-to (+ i indentation-offset))))
+            (insert indent-str))
 	  (forward-line)))
       (set-marker marker nil))))
 
@@ -637,7 +642,7 @@ Leave point in edit buffer."
   "Fontify code block between START and END using LANG's syntax.
 This function is called by Emacs' automatic fontification, as long
 as `org-src-fontify-natively' is non-nil."
-  (let ((modified (buffer-modified-p)))
+  (let ((modified (buffer-modified-p)) native-tab-width)
     (remove-text-properties start end '(face nil))
     (let ((lang-mode (org-src-get-lang-mode lang)))
       (when (fboundp lang-mode)
@@ -651,6 +656,7 @@ as `org-src-fontify-natively' is non-nil."
 	      ;; Add string and a final space to ensure property change.
 	      (insert string " "))
 	    (unless (eq major-mode lang-mode) (funcall lang-mode))
+            (setq native-tab-width tab-width)
             (font-lock-ensure)
 	    (let ((pos (point-min)) next)
 	      (while (setq next (next-property-change pos))
@@ -708,6 +714,21 @@ as `org-src-fontify-natively' is non-nil."
       (when (or (facep src-face) (listp src-face))
         (font-lock-append-text-property start end 'face src-face))
       (font-lock-append-text-property start end 'face 'org-block))
+    ;; Display native tab indentation characters as spaces
+    (save-excursion
+      (goto-char start)
+      (let ((indent-offset
+	     (if org-src-preserve-indentation 0
+	       (+ (progn (backward-char)
+                         (org-current-text-indentation))
+	          org-edit-src-content-indentation))))
+        (while (re-search-forward "^[ ]*\t" end t)
+          (let* ((b (and (eq indent-offset (move-to-column indent-offset))
+                         (point)))
+                 (e (progn (skip-chars-forward "\t") (point)))
+                 (s (and b (make-string (* (- e b) native-tab-width) ? ))))
+            (when (and b (< b e)) (add-text-properties b e `(display ,s)))
+            (forward-char)))))
     ;; Clear abbreviated link folding.
     (org-fold-region start end nil 'org-link)
     (add-text-properties
diff --git a/testing/lisp/test-org-src.el b/testing/lisp/test-org-src.el
index 2a45ba66e..c4309ccfc 100644
--- a/testing/lisp/test-org-src.el
+++ b/testing/lisp/test-org-src.el
@@ -304,11 +304,11 @@ This is a tab:\t.
 	(insert " Foo")
 	(org-edit-src-exit)
 	(buffer-string)))))
-  ;; Global indentation obeys `indent-tabs-mode' from the original
-  ;; buffer.
-  (should
+  ;; Global indentation does not obey `indent-tabs-mode' from the
+  ;; original buffer.
+  (should-not
    (string-match-p
-    "^\t+\s*argument2"
+    "\t"
     (org-test-with-temp-text
 	"
 - Item
@@ -323,14 +323,15 @@ This is a tab:\t.
 	(org-edit-special)
 	(org-edit-src-exit)
 	(buffer-string)))))
+  ;; Tab character is preserved
   (should
    (string-match-p
-    "^\s+argument2"
+    "\targument2"
     (org-test-with-temp-text
 	"
 - Item
   #+BEGIN_SRC emacs-lisp<point>
-    (progn\n      (function argument1\n\t\targument2))
+    (progn\n      (function argument1\n    \targument2))
   #+END_SRC"
       (setq-local indent-tabs-mode nil)
       (let ((org-edit-src-content-indentation 2)
@@ -338,43 +339,59 @@ This is a tab:\t.
 	(org-edit-special)
 	(org-edit-src-exit)
 	(buffer-string)))))
-  ;; Global indentation also obeys `tab-width' from original buffer.
+  ;; Indentation does not obey `tab-width' from org buffer.
   (should
    (string-match-p
-    "^\t\\{3\\}\s\\{2\\}argument2"
+    "^  \targument2"
     (org-test-with-temp-text
 	"
-- Item
-  #+BEGIN_SRC emacs-lisp<point>
+#+BEGIN_SRC emacs-lisp
   (progn
-    (function argument1
-              argument2))
-  #+END_SRC"
+    (list argument1\n  \t<point>argument2))
+#+END_SRC"
       (setq-local indent-tabs-mode t)
       (setq-local tab-width 4)
-      (let ((org-edit-src-content-indentation 0)
+      (let ((org-edit-src-content-indentation 2)
 	    (org-src-preserve-indentation nil))
 	(org-edit-special)
+        (setq-local indent-tabs-mode t)
+        (setq-local tab-width 8)
+        (lisp-indent-line)
 	(org-edit-src-exit)
 	(buffer-string)))))
+  ;; Tab characters are displayed with `tab-width' from the native
+  ;; edit buffer.
   (should
-   (string-match-p
-    "^\t\s\\{6\\}argument2"
+   (equal
+    10
     (org-test-with-temp-text
-	"
-- Item
-  #+BEGIN_SRC emacs-lisp<point>
+     "
+#+BEGIN_SRC emacs-lisp
   (progn
-    (function argument1
-              argument2))
-  #+END_SRC"
-      (setq-local indent-tabs-mode t)
-      (setq-local tab-width 8)
-      (let ((org-edit-src-content-indentation 0)
-	    (org-src-preserve-indentation nil))
-	(org-edit-special)
-	(org-edit-src-exit)
-	(buffer-string))))))
+    (list argument1\n  \t<point>argument2))
+#+END_SRC"
+     (setq-local indent-tabs-mode t)
+     (setq-local tab-width 4)
+     (let ((org-edit-src-content-indentation 2)
+	   (org-src-preserve-indentation nil))
+       (font-lock-ensure)
+       (current-column)))))
+  ;; The initial tab characters respect org's `tab-width'.
+  (should
+   (equal
+    10
+    (org-test-with-temp-text
+     "
+#+BEGIN_SRC emacs-lisp
+\t(progn
+\t  (list argument1\n\t\t<point>argument2))
+#+END_SRC"
+     (setq-local indent-tabs-mode t)
+     (setq-local tab-width 2)
+     (let ((org-edit-src-content-indentation 2)
+	   (org-src-preserve-indentation nil))
+       (font-lock-ensure)
+       (current-column))))))
 
 (ert-deftest test-org-src/footnote-references ()
   "Test editing footnote references."
-- 
2.41.0


[-- Attachment #3: 0001-org-src.el-Rename-internal-variable-for-clarity.patch --]
[-- Type: text/x-patch, Size: 3099 bytes --]

From bb16c48b8482104d9ea409e4260076ae08f42dc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= <sebastien.miquel@posteo.eu>
Date: Thu, 29 Jun 2023 14:37:09 +0200
Subject: [PATCH] org-src.el: Rename internal variable for clarity

* lisp/org-src.el (org-src--contents-for-write-back):
(org-src--edit-element): Rename `preserve-blank-line' to
`indent-current-empty-line'.  It is used to decide whether we should
indent the current empty line after a special edit.
---
 lisp/org-src.el | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 9e6031016..6fbb0b1c2 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -318,8 +318,8 @@ is 0.")
   "File name associated to Org source buffer, or nil.")
 (put 'org-src-source-file-name 'permanent-local t)
 
-(defvar-local org-src--preserve-blank-line nil)
-(put 'org-src--preserve-blank-line 'permanent-local t)
+(defvar-local org-src--curr-line-blank-indented nil)
+(put 'org-src--curr-line-blank-indented 'permanent-local t)
 
 (defun org-src--construct-edit-buffer-name (org-buffer-name lang)
   "Construct the buffer name for a source editing buffer.
@@ -473,7 +473,7 @@ Assume point is in the corresponding edit buffer."
                      (list (buffer-substring (point-min) eol)
                            (buffer-substring eol (point-max))))))
 	(write-back org-src--allow-write-back)
-        (preserve-blank-line org-src--preserve-blank-line)
+        (indent-current-empty-line org-src--curr-line-blank-indented)
         marker indent-str)
     (setq indent-str
           (with-temp-buffer
@@ -500,7 +500,7 @@ Assume point is in the corresponding edit buffer."
                     ;; If the current line is empty, we may
                     ;; want to indent it.
                     (and (eq (point) (marker-position marker))
-                         preserve-blank-line))
+                         indent-current-empty-line))
             (insert indent-str))
 	  (forward-line)))
       (set-marker marker nil))))
@@ -557,8 +557,6 @@ Leave point in edit buffer."
              (blank-line (save-excursion (beginning-of-line)
                                          (looking-at-p "^[[:space:]]*$")))
              (empty-line (and blank-line (looking-at-p "^$")))
-             (preserve-blank-line (or (and blank-line (not empty-line))
-                                      (and empty-line (= (+ block-ind content-ind) 0))))
 	     (preserve-ind
 	      (and (memq type '(example-block src-block))
 		   (or (org-element-property :preserve-indent datum)
@@ -608,7 +606,8 @@ Leave point in edit buffer."
 	(setq org-src--overlay overlay)
 	(setq org-src--allow-write-back write-back)
 	(setq org-src-source-file-name source-file-name)
-        (setq org-src--preserve-blank-line preserve-blank-line)
+        (setq org-src--curr-line-blank-indented (and blank-line
+                                                     (not empty-line)))
 	;; Start minor mode.
 	(org-src-mode)
 	;; Clear undo information so we cannot undo back to the
-- 
2.41.0