emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: "Sébastien Miquel" <sebastien.miquel@posteo.eu>
To: Ihor Radchenko <yantar92@posteo.net>, wolf <wolf@wolfsden.cz>
Cc: emacs-orgmode@gnu.org
Subject: Re: [BUG] Source block indentation does not work properly for yaml-mode [9.6.6 ( @ /home/user/.emacs.d/elpa/org-9.6.6/)]
Date: Sat, 17 Jun 2023 19:11:01 +0000	[thread overview]
Message-ID: <8d8642c9-ced3-b254-0f49-f7b9c06311ff@posteo.eu> (raw)
In-Reply-To: <87ttva8chx.fsf@localhost>

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

Hi Ihor, wolf,

Ihor Radchenko writes:
> Confirmed.
> This is caused by `org-src--contents-for-write-back' not adjusting
> blank line indentation in some cases.

I don't think that's the issue. In fact, applying your diff didn't
seem to solve the issue on my end.

Generally, if you edit the given yaml in a =C-c C-'= buffer and go
back to the org buffer with the default configuration, spaces will be
converted to tabs, because =indent-tabs-mode= is =t= in the org buffer.

I don't think there's much that we can do about it. We could try to
read the value of =indent-tabs-mode= in the native buffer and preserve
it in the org buffer, but then the org buffer would have mixed
indentation all over, and that's exactly what the current code tries to
avoid.

To fix the original issue, you can set =indent-tabs-mode= to =nil= in
your org files, or possibly set =org-preserve-indentation= to =t=
(untested).

> I feel that the whole approach we use now with preserve-fl, use-tabs?,
> and preserve-blank-line is overcomplicated. Maybe someone can explain
> why we need all these special cases? The code does not reveal a whole lot.

  - =use-tabs?= seems pretty straightforward, its purpose is to respect
    the value of =indent-tabs-mode= in the org buffer.
  - =preserve-fl= is an isolated issue, and only concerns LaTeX
    fragments. I will attach a test with the issue it solves with
    multiline LaTeX fragments. I think LaTeX fragments are particular
    because in the org buffer they do not need to start at the
    beginning of a line.
  - The =preserve-blank-line= part (committed by me) is quite abstruse.
    Its name does not make any sense !

    Originally, we did not try to reindent blank lines when writing
    back to the org buffer. I changed it so that when using
    =org-return=, the newline would get correctly indented, I think.
    Then I changed it again so that only the current blank line might
    get indented, see
    : https://list.orgmode.org/725763.1632663635@apollo2.minshall.org/T/
    The variable =preserve-blank-line= decides whether we should indent
    the current blank line (if it is empty, we should maybe not).


Here are three patches attached.
  1. Two tests : about editing LaTeX fragments, and preserving empty
     lines.
  2. Renaming of =preserve-blank-line=, for clarity.
  3. Two more tests testing the behaviour of =org-return= and
     indentation, with the default configuration. When writing these, I
     found the behaviour was buggy in one case, and modified
     =org-indent-line= to fix it.

Does that look alright to you ?

Regards,
-- 
Sébastien Miquel

[-- Attachment #2: 0001-test-org-src.el-Add-two-tests.patch --]
[-- Type: text/x-patch, Size: 2516 bytes --]

From 9613a54d20883e56301f987f5495b962f3763cad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= <sebastien.miquel@posteo.eu>
Date: Sat, 17 Jun 2023 17:00:51 +0200
Subject: [PATCH] test-org-src.el: Add two tests

* testing/lisp/test-org-src.el (test-org-src/preserve-empty-lines):
Test that empty lines are not indented.
(test-org-src/indented-latex-fragments): Test special edit of
multiline indented LaTeX fragment.
---
 testing/lisp/test-org-src.el | 52 ++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/testing/lisp/test-org-src.el b/testing/lisp/test-org-src.el
index 2a45ba66e..42edb364a 100644
--- a/testing/lisp/test-org-src.el
+++ b/testing/lisp/test-org-src.el
@@ -144,6 +144,47 @@ This is a tab:\t.
               (org-edit-src-exit)
               (buffer-string))))))
 
+(ert-deftest test-org-src/preserve-empty-lines ()
+  "Editing block preserves empty lines."
+  (should
+   (equal "
+#+begin_src emacs-lisp
+  The following line is empty
+
+  abc
+#+end_src"
+          (org-test-with-temp-text
+           "
+#+begin_src emacs-lisp
+  The following line is empty
+
+  abc<point>
+#+end_src"
+           (let ((org-edit-src-content-indentation 2)
+                 (org-src-preserve-indentation nil))
+             (org-edit-special)
+             (org-edit-src-exit)
+             (buffer-string)))))
+  (should
+   (equal "
+#+begin_src emacs-lisp
+  The following line is empty
+
+  abc
+#+end_src"
+          (org-test-with-temp-text
+           "
+#+begin_src emacs-lisp
+  The following line is empty
+<point>
+  abc
+#+end_src"
+           (let ((org-edit-src-content-indentation 2)
+                 (org-src-preserve-indentation nil))
+             (org-edit-special)
+             (org-edit-src-exit)
+             (buffer-string)))))  )
+
 (ert-deftest test-org-src/coderef-format ()
   "Test `org-src-coderef-format' specifications."
   ;; Regular tests in a src block, an example block and an edit
@@ -376,6 +417,17 @@ This is a tab:\t.
 	(org-edit-src-exit)
 	(buffer-string))))))
 
+(ert-deftest test-org-src/indented-latex-fragments ()
+  "Test editing multiline indented LaTeX fragment."
+  (should
+   (equal
+    "- Item $abc\n  efg$"
+    (org-test-with-temp-text
+     "- Item $abc<point>\n  efg$"
+     (org-edit-special)
+     (org-edit-src-exit)
+     (buffer-string)))))
+
 (ert-deftest test-org-src/footnote-references ()
   "Test editing footnote references."
   ;; Error when there is no definition to edit.
-- 
2.41.0


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

From cb7e3fb59109d2e3ea4fab72ae2cd69f5b1efa08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= <sebastien.miquel@posteo.eu>
Date: Sat, 17 Jun 2023 17:10:53 +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-blank-line'. It is used to decide whether we should
indent the current blank line after a special edit.
---
 lisp/org-src.el | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index f15ba8e99..f0982ea12 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--indent-current-blank-line nil)
+(put 'org-src--indent-current-blank-line '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-blank-line org-src--indent-current-blank-line)
         marker)
     (with-current-buffer write-back-buf
       ;; Reproduce indentation parameters from source buffer.
@@ -493,7 +493,7 @@ Assume point is in the corresponding edit buffer."
 	  (skip-chars-forward " \t")
           (when (or (not (eolp))                               ; not a blank line
                     (and (eq (point) (marker-position marker)) ; current line
-                         preserve-blank-line))
+                         indent-current-blank-line))
 	    (let ((i (current-column)))
 	      (delete-region (line-beginning-position) (point))
 	      (indent-to (+ i indentation-offset))))
@@ -552,8 +552,9 @@ 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))))
+             (indent-current-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)
@@ -603,7 +604,7 @@ 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--indent-current-blank-line indent-current-blank-line)
 	;; Start minor mode.
 	(org-src-mode)
 	;; Clear undo information so we cannot undo back to the
-- 
2.41.0


[-- Attachment #4: 0001-org.el-org-indent-line-Fix-to-the-indentation-inside.patch --]
[-- Type: text/x-patch, Size: 4240 bytes --]

From c5d9197700257f1505a70c0cfee0aa83619a4399 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= <sebastien.miquel@posteo.eu>
Date: Sat, 17 Jun 2023 19:07:50 +0200
Subject: [PATCH] org.el (org-indent-line): Fix to the indentation inside src
 blocks

* lisp/org.el (org-indent-line): When indenting a line inside a src
block, preindent it with the common block indentation before calling
native indentation.
* testing/lisp/test-org-src.el (test-org-src/org-return-indents): Test
indentation inside src blocks, when calling `org-return'.
---
 lisp/org.el                  | 19 +++++++++++++------
 testing/lisp/test-org-src.el | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 4273385bf..38d13f111 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -19083,20 +19083,27 @@ Also align node properties according to `org-property-format'."
 		     (org-with-point-at (org-element-property :end element)
 		       (skip-chars-backward " \t\n")
 		       (line-beginning-position))))
-             ;; At the beginning of a blank line, do some preindentation.  This
-             ;; signals org-src--edit-element to preserve the indentation on exit
-             (when (and (looking-at-p "^[[:space:]]*$")
-                        (not org-src-preserve-indentation))
-               (let (block-content-ind some-ind)
+             ;; Do some preindentation, to add the common block
+             ;; indentation to the current line.
+             (when (not org-src-preserve-indentation)
+               (let ((current-ind (org-current-text-indentation))
+                     block-content-ind some-ind)
                  (org-with-point-at (org-element-property :begin element)
                    (setq block-content-ind (+ (org-current-text-indentation)
                                               org-edit-src-content-indentation))
+                   ;; Check that the first line of the block has the
+                   ;; minimal indentation
                    (forward-line)
 		   (save-match-data (re-search-forward "^[ \t]*\\S-" nil t))
                    (backward-char)
                    (setq some-ind (if (looking-at-p "#\\+end_src")
                                       block-content-ind (org-current-text-indentation))))
-                 (indent-line-to (min block-content-ind some-ind))))
+                 ;; If the current line is the first one and does not
+                 ;; have the minimal indentation, we do not preindent,
+                 ;; since it could break the relative indentation with
+                 ;; respect to the following lines
+                 (when (< current-ind (min block-content-ind some-ind))
+                   (indent-line-to (min block-content-ind some-ind)))))
 	     (org-babel-do-key-sequence-in-edit-buffer (kbd "TAB")))
 	    (t
 	     (let ((column (org--get-expected-indentation element nil)))
diff --git a/testing/lisp/test-org-src.el b/testing/lisp/test-org-src.el
index 42edb364a..4d943927d 100644
--- a/testing/lisp/test-org-src.el
+++ b/testing/lisp/test-org-src.el
@@ -144,6 +144,40 @@ This is a tab:\t.
               (org-edit-src-exit)
               (buffer-string))))))
 
+(ert-deftest test-org-src/org-return-indents ()
+  "Calling `org-return' indents newline."
+  (should
+   (equal "
+#+begin_src emacs-lisp
+  (or c1\n      \n      c2)
+#+end_src"
+          (org-test-with-temp-text
+           "
+#+begin_src emacs-lisp
+  (or c1<point>
+      c2)
+#+end_src"
+           (let ((org-edit-src-content-indentation 2)
+                 (org-src-tab-acts-natively t)
+                 (org-src-preserve-indentation nil))
+             (org-return t)
+             (buffer-string)))))
+  (should
+   (equal "
+#+begin_src emacs-lisp
+  (or c1
+      c2)
+#+end_src"
+          (org-test-with-temp-text
+           "
+#+begin_src emacs-lisp
+  (or c1<point> c2)
+#+end_src"
+           (let ((org-edit-src-content-indentation 2)
+                 (org-src-preserve-indentation nil))
+             (org-return t)
+             (buffer-string))))))
+
 (ert-deftest test-org-src/preserve-empty-lines ()
   "Editing block preserves empty lines."
   (should
-- 
2.41.0


  reply	other threads:[~2023-06-17 19:12 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-11 22:33 [BUG] Source block indentation does not work properly for yaml-mode [9.6.6 ( @ /home/user/.emacs.d/elpa/org-9.6.6/)] wolf
2023-06-14 12:16 ` Ihor Radchenko
2023-06-17 19:11   ` Sébastien Miquel [this message]
2023-06-18 11:16     ` Ihor Radchenko
2023-06-19  8:43       ` Sébastien Miquel
2023-06-19 11:05         ` Ihor Radchenko
2023-06-19 15:32           ` Sébastien Miquel
2023-06-20 10:02             ` Ihor Radchenko
2023-06-21  5:46               ` Sébastien Miquel
2023-06-25 10:46               ` Ihor Radchenko
2023-06-26 11:14               ` Sébastien Miquel
2023-06-26 11:45                 ` Sébastien Miquel
2023-06-26 11:52                 ` Ihor Radchenko
2023-06-26 12:15                   ` Sébastien Miquel
2023-06-26 12:44                     ` Ihor Radchenko
2023-06-27  8:54                       ` Sébastien Miquel
2023-06-28  9:21                         ` Ihor Radchenko
2023-06-29 15:54                           ` Sébastien Miquel
2023-06-30 11:43                             ` Ihor Radchenko
2023-06-30 20:27                               ` Sébastien Miquel
2023-07-01 11:07                                 ` Ihor Radchenko
2023-07-01 17:17                                   ` Sébastien Miquel
2023-07-03  9:58                                     ` Ihor Radchenko
2023-07-03 12:49                                       ` Sébastien Miquel
2023-07-03 13:05                                         ` Ihor Radchenko
2023-07-03 13:48                                           ` Sébastien Miquel
2023-07-04 10:41                                             ` Ihor Radchenko
2023-07-06 11:01                                               ` Sébastien Miquel
2023-07-07  9:26                                                 ` Ihor Radchenko
2023-07-07  9:54                                                   ` Ihor Radchenko
2023-07-07 13:21                                                     ` Sébastien Miquel
2023-07-08  8:44                                                       ` Ihor Radchenko
2023-07-09 11:10                                                         ` Sébastien Miquel
2023-07-10  8:22                                                           ` Ihor Radchenko
2023-07-07  9:31                                                 ` [BUG] org-list-struct-apply-struct overrides src block indentation (was: [BUG] Source block indentation does not work properly for yaml-mode [9.6.6 ( @ /home/user/.emacs.d/elpa/org-9.6.6/)]) Ihor Radchenko
2023-07-07 13:43                                                   ` Sébastien Miquel
2023-07-08  9:06                                                     ` Ihor Radchenko

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

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8d8642c9-ced3-b254-0f49-f7b9c06311ff@posteo.eu \
    --to=sebastien.miquel@posteo.eu \
    --cc=emacs-orgmode@gnu.org \
    --cc=wolf@wolfsden.cz \
    --cc=yantar92@posteo.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 public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).