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

2023-07-10 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Subject: [PATCH] test-org-src.el: Work around `current-column' bug in older
>  emacs
>
> * testing/lisp/test-org-src.el (test-org-src/indented-blocks): Work
> around `current-column' not working in the presence of display strings
> in older emacs.

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

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-07-09 Thread Sébastien Miquel


Ihor Radchenko writes:

We should probably reserve the workaround to Emacs 28 and older and
eventually remove it when Org drops Emacs 28 support.


Ok.


I tested using Emacs 28 and 27 and your patch is passing all the tests.


Thanks.

--
Sébastien Miquel
From 6ea37a3041fb3266e94af0bfce29aa76f6c4439d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
Date: Fri, 7 Jul 2023 13:58:17 +0200
Subject: [PATCH] test-org-src.el: Work around `current-column' bug in older
 emacs

* testing/lisp/test-org-src.el (test-org-src/indented-blocks): Work
around `current-column' not working in the presence of display strings
in older emacs.
---
 testing/lisp/test-org-src.el | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/testing/lisp/test-org-src.el b/testing/lisp/test-org-src.el
index a634d85e8..ebf8d8569 100644
--- a/testing/lisp/test-org-src.el
+++ b/testing/lisp/test-org-src.el
@@ -416,7 +416,13 @@ This is a tab:\t.
  (let ((org-edit-src-content-indentation 2)
 	   (org-src-preserve-indentation nil))
(font-lock-ensure)
-   (current-column)
+   ;;  `current-column' will not work with older versions of emacs
+   ;; before commit 4243747b1b8: Fix 'current-column' in the
+   ;; presence of display strings
+   (if (<= emacs-major-version 28)
+   (+ (progn (backward-char) (length (get-text-property (point) 'display)))
+  (current-column))
+ (current-column))
   ;; The initial tab characters respect org's `tab-width'.
   (should
(equal
@@ -432,7 +438,10 @@ This is a tab:\t.
  (let ((org-edit-src-content-indentation 2)
 	   (org-src-preserve-indentation nil))
(font-lock-ensure)
-   (current-column))
+   (if (<= emacs-major-version 28)
+   (+ (progn (backward-char) (length (get-text-property (point) 'display)))
+  (current-column))
+ (current-column)))
 
 (ert-deftest test-org-src/indented-latex-fragments ()
   "Test editing multiline indented LaTeX fragment."
-- 
2.41.0



Re: [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/)])

2023-07-08 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Ihor Radchenko writes:
>> The cause is the following line in `org-list-struct-apply-struct'
>> [[file:~/Git/org-mode/lisp/org-list.el::indent-line-to  (+ 
>> (org-current-text-indentation) delta)))]]
>>
>> It calls `indent-line-to' that replaces spaces with tabs according to
>> current value of indent-tabs-mode in Org buffer.
>
> There are a few other instances of `indent-line-to` in the code. I
> guess the right fix is to un-obsolete `org-indent-line-to`, use it and
> make a special case if point is in a src block.

Yup. Sounds reasonable. Although we should not use it blindly; just
where necessary. Checking for element at point will create overheads.

> This use case is
> uncommon and not really compatible with `org-src-preserve-indentation`
> though.

May you please elaborate?

> Somewhat more common possibility: say one has a src block at 0
> indentation, and wants to make it part of an org list. Is there any
> proper org way to do this ? I can use `indent-rigidly`, but again,
> this might break an org-src indentation. No easy fix to this beside
> providing a simple org version of `indent-rigidly`.

I recall struggling with it and ending up with adding spaces manually to
the first src line and then using indent-region.

`indent-line-function' technically allows force indent, if
`tab-always-indent' is set to t, but we have  bound to org-cycle,
which will take precedence when at #+begin_src.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-07-08 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Ihor Radchenko writes:
>> Sebastien, it looks like one of the tests is failing on the older Emacs:
>> https://builds.sr.ht/~bzg/job/1020247
>
> Does this specify anywhere what version of emacs it is using ?

Yup. In the full log https://builds.sr.ht/query/log/1020247/build/log
It is Emacs 28.2. Older Emacs versions are also failing.

>> Most likely, because `current-column' did not take into account 'display
>> property until recently.
>
> Here's a workaround. I think I found what emacs commit makes it work,
> but I haven't been able to test it with an older emacs version.

We should probably reserve the workaround to Emacs 28 and older and
eventually remove it when Org drops Emacs 28 support.

> Subject: [PATCH] test-org-src.el: Work around `current-column' bug in older
>  emacs
>
> * testing/lisp/test-org-src.el (test-org-src/indented-blocks): Work
> around `current-column' not working in the presence of display strings
> in older emacs.

I tested using Emacs 28 and 27 and your patch is passing all the tests.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [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/)])

2023-07-07 Thread Sébastien Miquel



Ihor Radchenko writes:

The cause is the following line in `org-list-struct-apply-struct'
[[file:~/Git/org-mode/lisp/org-list.el::indent-line-to  (+ 
(org-current-text-indentation) delta)))]]

It calls `indent-line-to' that replaces spaces with tabs according to
current value of indent-tabs-mode in Org buffer.


There are a few other instances of `indent-line-to` in the code. I
guess the right fix is to un-obsolete `org-indent-line-to`, use it and
make a special case if point is in a src block. This use case is
uncommon and not really compatible with `org-src-preserve-indentation`
though.

Somewhat more common possibility: say one has a src block at 0
indentation, and wants to make it part of an org list. Is there any
proper org way to do this ? I can use `indent-rigidly`, but again,
this might break an org-src indentation. No easy fix to this beside
providing a simple org version of `indent-rigidly`.

The issues above do not seem too bad. They are uncommon, and an
indent-region call should fix the indentation.

--
Sébastien Miquel



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

2023-07-07 Thread Sébastien Miquel


Ihor Radchenko writes:

Sebastien, it looks like one of the tests is failing on the older Emacs:
https://builds.sr.ht/~bzg/job/1020247


Does this specify anywhere what version of emacs it is using ?


Most likely, because `current-column' did not take into account 'display
property until recently.


Here's a workaround. I think I found what emacs commit makes it work,
but I haven't been able to test it with an older emacs version.

--
Sébastien MiquelFrom 81b33f16ad2e2b2cff20110ff1dafefbc348f9dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
Date: Fri, 7 Jul 2023 13:58:17 +0200
Subject: [PATCH] test-org-src.el: Work around `current-column' bug in older
 emacs

* testing/lisp/test-org-src.el (test-org-src/indented-blocks): Work
around `current-column' not working in the presence of display strings
in older emacs.
---
 testing/lisp/test-org-src.el | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/testing/lisp/test-org-src.el b/testing/lisp/test-org-src.el
index a634d85e8..5a3af8bb3 100644
--- a/testing/lisp/test-org-src.el
+++ b/testing/lisp/test-org-src.el
@@ -416,7 +416,11 @@ This is a tab:\t.
  (let ((org-edit-src-content-indentation 2)
 	   (org-src-preserve-indentation nil))
(font-lock-ensure)
-   (current-column)
+   ;; Replacement for `current-column', since it doesn't work with
+   ;; older versions of emacs before commit 4243747b1b8: Fix
+   ;; 'current-column' in the presence of display strings
+   (+ (progn (backward-char) (length (get-text-property (point) 'display)))
+  (current-column))
   ;; The initial tab characters respect org's `tab-width'.
   (should
(equal
@@ -432,7 +436,8 @@ This is a tab:\t.
  (let ((org-edit-src-content-indentation 2)
 	   (org-src-preserve-indentation nil))
(font-lock-ensure)
-   (current-column))
+   (+ (progn (backward-char) (length (get-text-property (point) 'display)))
+  (current-column)))
 
 (ert-deftest test-org-src/indented-latex-fragments ()
   "Test editing multiline indented LaTeX fragment."
-- 
2.41.0



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

2023-07-07 Thread Ihor Radchenko
Ihor Radchenko  writes:

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

Sebastien, it looks like one of the tests is failing on the older Emacs:
https://builds.sr.ht/~bzg/job/1020247

Most likely, because `current-column' did not take into account 'display
property until recently.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



[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/)])

2023-07-07 Thread Ihor Radchenko
Using a similar example, another indentation bug has been revealed:

1. emacs -Q
2. Create an Org file with the following contents:

- 1
  - 2
- 3
  - 4
- 5
  #+begin_src yaml
a:
  b:
c:
  d:
  #+end_src

Everything indented using spaces, including "d:" that uses 8 spaces,
which should remain spaces since yaml-mode uses spaces for indentation.

3. M-x whitespace-mode
4. Move point to item 5 and press M- 3 times
5. Observe "d:" indented using two tabs with all the spaces replaced by tabs.

The cause is the following line in `org-list-struct-apply-struct'
[[file:~/Git/org-mode/lisp/org-list.el::indent-line-to (+ 
(org-current-text-indentation) delta)))]]

It calls `indent-line-to' that replaces spaces with tabs according to
current value of indent-tabs-mode in Org buffer.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-07-07 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Ihor Radchenko writes:
>> May you now rebase the patch onto the latest main, add a Link: to this
>> discussion to the commit message, and apply the attached extra comments?
>
> Here it is. Thanks for helping with this.

Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=2e2ed4055
The original bug can no longer be reproduced.
Fixed.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-07-06 Thread Sébastien Miquel


Ihor Radchenko writes:

May you now rebase the patch onto the latest main, add a Link: to this
discussion to the commit message, and apply the attached extra comments?


Here it is. Thanks for helping with this.

--
Sébastien MiquelFrom 7906d7b7fa2d376e95156ab7177494f2cececaff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
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.el (org-indent-line): Simplify native indentation inside
src block.  Ensure we add the org indentation if the line is empty.
* 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): Display the native indentation tab
characters with a fixed width, according to the native tab width
value, to preserve vertical alignement in the org buffer.
* 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 an eventual org part, and the native part.

Link: https://list.orgmode.org/87a5wcez7e.fsf@localhost/T/#t
---
 lisp/org-macs.el |  9 +++--
 lisp/org-src.el  | 57 ---
 lisp/org.el  | 23 ---
 testing/lisp/test-org-src.el | 75 ++--
 4 files changed, 94 insertions(+), 70 deletions(-)

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index d6879e8cf..aa5c4845e 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -402,9 +402,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 317682844..e1f7d50dc 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -326,9 +326,6 @@ 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)
-
 (defun org-src--construct-edit-buffer-name (org-buffer-name lang)
   "Construct the buffer name for a source editing buffer.
 Format is \"*Org Src ORG-BUFFER-NAME[ LANG ]*\"."
@@ -481,12 +478,17 @@ 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)
-marker)
+marker indent-str)
+;; Compute the exact sequence of tabs and spaces used to indent up
+;; to `indentation-offset' in the Org buffer.
+(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))
@@ -496,15 +498,14 @@ 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
- preserve-blank-line))
-	(let ((i (current-column)))
-	  (delete-region (line-beginning-position) (point))
-	  

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

2023-07-04 Thread Ihor Radchenko
Sébastien Miquel  writes:

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

Thanks! This looks good and also fixes the original issue raised in this
thread.

May you now rebase the patch onto the latest main, add a Link: to this
discussion to the commit message, and apply the attached extra comments?

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 20e0b598c..e1f7d50dc 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -479,6 +479,8 @@ (defun org-src--contents-for-write-back (write-back-buf)
(buffer-substring eol (point-max))
 	(write-back org-src--allow-write-back)
 marker indent-str)
+;; Compute the exact sequence of tabs and spaces used to indent up
+;; to `indentation-offset' in the Org buffer.
 (setq indent-str
   (with-temp-buffer
 ;; Reproduce indentation parameters from org buffer.
@@ -500,6 +502,9 @@ (defun org-src--contents-for-write-back (write-back-buf)
 ;; first line.
 (when preserve-fl (forward-line))
 (while (not (eobp))
+  ;; Keep empty src lines empty, even when src block is
+  ;; indented on Org side.
+  ;; See https://list.orgmode.org/725763.1632663...@apollo2.minshall.org/T/
   (when (not (eolp)) (insert indent-str)) ; not an empty line
 	  (forward-line)))
   (set-marker marker nil

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 


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

2023-07-03 Thread Sébastien Miquel


Ihor Radchenko writes:

For the second scenario, no special treatment of current line is needed.
For the first scenario, why do we need to do it all the way in
`org-src--contents-for-write-back'? Why not directly in
`org-indent-line'?


Ah, yes, that is much better.

--
Sébastien MiquelFrom 9d31a71bc0ab7cfd466ecad60037d00c62bdd9f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
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.el (org-indent-line): Simplify native indentation inside
src block.  Ensure we add the org indentation if the line is empty.
* 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  | 52 ++---
 lisp/org.el  | 23 ---
 testing/lisp/test-org-src.el | 75 ++--
 4 files changed, 89 insertions(+), 70 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..2beb49e63 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -318,9 +318,6 @@ 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)
-
 (defun org-src--construct-edit-buffer-name (org-buffer-name lang)
   "Construct the buffer name for a source editing buffer.
 Format is \"*Org Src ORG-BUFFER-NAME[ LANG ]*\"."
@@ -473,12 +470,15 @@ 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)
-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 +488,11 @@ 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
- preserve-blank-line))
-	(let ((i (current-column)))
-	  (delete-region (line-beginning-position) (point))
-	  (indent-to (+ i indentation-offset
+  (when (not (eolp)) (insert indent-str)) ; not an empty line
 	  (forward-line)))
   (set-marker marker nil
 
@@ -549,11 +545,6 @@ Leave point in edit 

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

2023-07-03 Thread Ihor Radchenko
Sébastien Miquel  writes:

>> May you please provide an example when it is necessary?
>> `org-indent-line' will run `org-babel-do-key-sequence-in-edit-buffer', so
>> it should still use `org-src--contents-for-write-back' and will not
>> modify the org buffer text directly.
>
> You're at the end of a line, you press =org-return-and-indent=.
>   1. It adds a newline character.
>   2. =org-indent-line= adds the org indentation, _before_ calling
>  =org-babel-do-key-sequence-in-edit-buffer=

I missed this:

 ;; At the beginning of a blank line, do some preindentation.  This
 ;; signals org-src--edit-element to preserve the indentation on 
exit

>   3. The native edit call removes the common indentation, before
>  calling tab in the native buffer.
>   4. Calling tab in the native buffer possibly does nothing.
>   5. =org-src--contents-for-write-back= sees the current line is empty,
>  but it should indent it (with org indentation) nonetheless.

Ok. I understand better now (I think).

We are talking about

\t#+begin_src bash
\tcd foo
\t#+end_src

and user pressing 
Then, the expected result is

\t#+begin_src bash
\tcd foo
\t
\t#+end_src

with  being aligned with "cd foo" above.

Alternatively,

\t#+begin_src emacs-lisp
\t(when t
\t#+end_src

 should yield indentation in src block as well

\t#+begin_src emacs-lisp
\t(when t
\t  
\t#+end_src

-

For the second scenario, no special treatment of current line is needed.
For the first scenario, why do we need to do it all the way in
`org-src--contents-for-write-back'? Why not directly in
`org-indent-line'?

>> Before your change, all the blank non-empty lines were unconditionally
>> removed. After your change, the first such line is removed and the
>> function returns nil without continuing.
>
> I don't understand. With this change, the function only stops if it
> finds a non blank line with less than n indentation (same as before).
> When a blank line with less than n indentation is found, it is emptied
> (same as before), and execution continues.

Never mind this. I misread the code. Thought that `throw' is called on
blank lines.

>> Since not removing blank lines is intentional after the change, why
>> doing it on a single line that is indented less than N?
>
> Are you advocating for computing N using blank lines as well ?

No. It was a misunderstanding.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-07-03 Thread Sébastien Miquel



Ihor Radchenko writes:

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.


May you please provide an example when it is necessary?
`org-indent-line' will run `org-babel-do-key-sequence-in-edit-buffer', so
it should still use `org-src--contents-for-write-back' and will not
modify the org buffer text directly.


You're at the end of a line, you press =org-return-and-indent=.
 1. It adds a newline character.
 2. =org-indent-line= adds the org indentation, _before_ calling
=org-babel-do-key-sequence-in-edit-buffer=
 3. The native edit call removes the common indentation, before
calling tab in the native buffer.
 4. Calling tab in the native buffer possibly does nothing.
 5. =org-src--contents-for-write-back= sees the current line is empty,
but it should indent it (with org indentation) nonetheless.



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


This function is actually confusing both before and after the change.
According to the docstring:

 When optional argument N is a positive integer, remove exactly
 that much characters from indentation, if possible.

But the function can actually remove less than N characters.

Before your change, all the blank non-empty lines were unconditionally
removed. After your change, the first such line is removed and the
function returns nil without continuing.


I don't understand. With this change, the function only stops if it
finds a non blank line with less than n indentation (same as before).
When a blank line with less than n indentation is found, it is emptied
(same as before), and execution continues.


* lisp/org-macs.el (org-do-remove-indentation): Preserve
indentation (spaces vs tabs) past the common indentation to remove.
Do not empty blank lines.


Since not removing blank lines is intentional after the change, why
doing it on a single line that is indented less than N?


Are you advocating for computing N using blank lines as well ?

 1. It isn't consistent with the previous behaviour.
 2. If I mistakenly add a space to an empty line in a src block, an
edit-special round trip will add indentation to every line.

Is there any benefit ?

--
Sébastien Miquel



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

2023-07-03 Thread Ihor Radchenko
Sébastien Miquel  writes:

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

Ok.

I am not that much worried about consistency with inline src blocks.
Rather do not like the mix of org-src buffer local variables and
checking the block type. But we can leave this refactoring to another
day. It is not just about preserve-fl. (also indentation-offset)

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

May you please provide an example when it is necessary?
`org-indent-line' will run `org-babel-do-key-sequence-in-edit-buffer', so
it should still use `org-src--contents-for-write-back' and will not
modify the org buffer text directly.

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

This function is actually confusing both before and after the change.
According to the docstring:

When optional argument N is a positive integer, remove exactly
that much characters from indentation, if possible.

But the function can actually remove less than N characters.

Before your change, all the blank non-empty lines were unconditionally
removed. After your change, the first such line is removed and the
function returns nil without continuing.

> * lisp/org-macs.el (org-do-remove-indentation): Preserve
> indentation (spaces vs tabs) past the common indentation to remove.
> Do not empty blank lines.

Since not removing blank lines is intentional after the change, why
doing it on a single line that is indented less than N?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-07-01 Thread Sébastien Miquel


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 MiquelFrom ecc87b4a75dec7ff8fba4c86635ba3cdb43444ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
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 

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

2023-07-01 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Ihor Radchenko writes:
>> But why do we need to avoid indenting empty lines?
>
> In the following link, Greg Minshal argues for preserving empty lines:
> https://list.orgmode.org/725763.1632663...@apollo2.minshall.org/T/

Thanks for the reference!
I can see that non-empty blank line below might be annoying for some users.

\t#+begin_src lang
\t  line1
\t  line2
^\t  $
\t#+end_src

However, I do not see why Org should clear blank lines created by the
lang-mode itself

\t#+begin_src lang
\t  line1:
\t  \tline2:
^\t \t\t$
\t#+end_src

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 code in your patch is confusing, considering the above
considerations.

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

Also,

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

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-30 Thread Sébastien Miquel



Ihor Radchenko writes:

But why do we need to avoid indenting empty lines?


In the following link, Greg Minshal argues for preserving empty lines:
https://list.orgmode.org/725763.1632663...@apollo2.minshall.org/T/

(CCing the mailing list)

--
Sébastien Miquel




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

2023-06-30 Thread Ihor Radchenko
Sébastien Miquel  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?

>>> +  (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))
>> Do we still need this dance with special case for current line?
>
> Yes. This is fragile, but what it does is, if the line from which the
> original edit was called was blank and not empty, and the line from
> which the edit is ended is empty, we assume these two lines are the
> same, and we add the common block indentation to this empty line.
> This, and the pre-indentation in =org-indent-line=, make `TAB` work
> to indent an empty line.

But why do we need to avoid indenting empty lines?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-29 Thread Sébastien Miquel

Hi Ihor,

Thank you for the feedback.

Ihor Radchenko writes:

+  ;; Apply WRITE-BACK function on edit buffer contents.
+  (goto-char (point-min))
+  (when (functionp write-back) (save-excursion (funcall write-back)))
(set-marker marker nil

`save-excursion' is no longer necessary here.


Done.


  (defun org-src--edit-element
@@ -1150,7 +1149,14 @@ Throw an error when not at such a table."
 (lambda ()
 ;; Blank lines break things, replace with a single newline.
 (while (re-search-forward "\n[ \t]*\n" nil t) (replace-match "\n"))
-;; If within a table a newline would disrupt the structure,
+ ;; Trim contents.

It would be nice to have a bit more context about the purpose in the
comment here.


I was confused. We need only trim the beginning of the result, for the
indentation added by `org-src--contents-for-write-back'. I've added an
explanation.


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

Do we still need this dance with special case for current line?


Yes. This is fragile, but what it does is, if the line from which the
original edit was called was blank and not empty, and the line from
which the edit is ended is empty, we assume these two lines are the
same, and we add the common block indentation to this empty line.
This, and the pre-indentation in =org-indent-line=, make `TAB` work
to indent an empty line.

The logic in =org-edit-element= can now be simplified though, see the
third patch attached.


+;; Display tab indentation characters preceded by spaces as spaces
+(unless org-src-preserve-indentation

unless? Don't we rather want to preserve the original indentation
alignment when `org-src-preserve-indentation' is t?

+  (save-excursion
+(goto-char start)
+(while (re-search-forward "^[ ]+\\([\t]+\\)" end t)

Why just tabs at indentation? It would make sense to preserve width of
all the tabs.

+  (let* ((b (match-beginning 1))
+ (e (match-end 1))
+ (s (make-string (* (- e b) native-tab-width) ? )))
+(add-text-properties b e `(display ,s))

Will the actual tab width be always equal to native-tab-width? What
about tab stops? May it be more reliable to use different of column
numbers in the src mode buffer after/before the tab?


I was trying to be conservative. When =org-src-preserve-indentation=
is t, I guess we can do the same, for consistency.

As you say, the tabs at the beginning of indentation are the only ones
we can be somewhat sure of the width, if we assume the native
indentation was done sanely (using tabs then spaces). I'm inclined to
only deal with those.

To get the true tab widths, we'd need to get the columns numbers in
yet a third different buffer, with the common indentation removed. I
don't think that's worth it.

Anyway, what I wrote doesn't deal correctly with the case where the
org indentation may use tabs. I've updated the patch: I use the value
of what should be the org indentation, and only change the display of
the following consecutive tabs.


@@ -318,19 +318,21 @@ This is a tab:\t.
argument2))
#+END_SRC"
(setq-local indent-tabs-mode t)
-  (let ((org-edit-src-content-indentation 2)
+  (let ((tab-width 8)
+(org-edit-src-content-indentation 2)

Why is setting tab-width necessary? 8 is the default value.


Removed, though I have left an instance of =(setq-local tab-width 8)=,
for clarity.


#+BEGIN_SRC emacs-lisp
-(progn\n  (function argument1\n\t\targument2))
+(progn\n  (function argument1\n\targument2))

I think it would be a bit more readable to convert this string into
actual multi-line, where the alignment is visible when reading the test
source.


I've left it this way. I think the tests using tab characters are
often written this way, so as to be understandable without whitespace
mode.

--
Sébastien MiquelFrom d024fe96ad889097d025a87dae3316acc44299f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
Date: Sun, 25 Jun 2023 13:38:21 +0200
Subject: [PATCH] org-src--contents-for-write-back: simplify the case of LaTeX
 fragments

* lisp/org-src.el (org-src--contents-for-write-back): Extract special
case logic for LaTeX fragments.
(org-edit-latex-fragment): Trim the contents to be inserted in the
original buffer.
---
 lisp/org-src.el | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index f15ba8e99..121e59241 100644
--- a/lisp/org-src.el
+++ 

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

2023-06-28 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Here are two patches, the first that removes the special case logic
> for LaTeX fragments, and the second that implements what you suggest,
> and applies on top of the first one. Does that look ok to you ?

Thanks!

> +  ;; Apply WRITE-BACK function on edit buffer contents.
> +  (goto-char (point-min))
> +  (when (functionp write-back) (save-excursion (funcall write-back)))
>(set-marker marker nil

`save-excursion' is no longer necessary here.

>  (defun org-src--edit-element
> @@ -1150,7 +1149,14 @@ Throw an error when not at such a table."
> (lambda ()
>;; Blank lines break things, replace with a single newline.
>(while (re-search-forward "\n[ \t]*\n" nil t) (replace-match "\n"))
> -  ;; If within a table a newline would disrupt the structure,
> + ;; Trim contents.

It would be nice to have a bit more context about the purpose in the
comment here.

> Subject: [PATCH] org-src.el: Use native value of `indent-tabs-mode' for
>  indentation
>
> * 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.

> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index 5c272c7f5..5a1030c42 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
>  ...
> -   (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))

Do we still need this dance with special case for current line?

> +;; Display tab indentation characters preceded by spaces as spaces
> +(unless org-src-preserve-indentation

unless? Don't we rather want to preserve the original indentation
alignment when `org-src-preserve-indentation' is t?

> +  (save-excursion
> +(goto-char start)
> +(while (re-search-forward "^[ ]+\\([\t]+\\)" end t)

Why just tabs at indentation? It would make sense to preserve width of
all the tabs.

> +  (let* ((b (match-beginning 1))
> + (e (match-end 1))
> + (s (make-string (* (- e b) native-tab-width) ? )))
> +(add-text-properties b e `(display ,s))

Will the actual tab width be always equal to native-tab-width? What
about tab stops? May it be more reliable to use different of column
numbers in the src mode buffer after/before the tab?

> @@ -318,19 +318,21 @@ This is a tab:\t.
>argument2))
>#+END_SRC"
>(setq-local indent-tabs-mode t)
> -  (let ((org-edit-src-content-indentation 2)
> +  (let ((tab-width 8)
> +(org-edit-src-content-indentation 2)

Why is setting tab-width necessary? 8 is the default value.

>#+BEGIN_SRC emacs-lisp
> -(progn\n  (function argument1\n\t\targument2))
> +(progn\n  (function argument1\n\targument2))

I think it would be a bit more readable to convert this string into
actual multi-line, where the alignment is visible when reading the test
source.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-27 Thread Sébastien Miquel


Ihor Radchenko writes:

This is not a problem. We can just apply appropriate 'display property
in `org-src-font-lock-fontify-block', manually replacing the tab with
appropriate number of spaces (as in the origin buffer).


Ok, that works, thanks.

Here are two patches, the first that removes the special case logic
for LaTeX fragments, and the second that implements what you suggest,
and applies on top of the first one. Does that look ok to you ?

NB: Calling newline-and-indent from the middle of a line is currently
broken, but that has nothing to do with this change.

--
Sébastien MiquelFrom d2a86d9011455172e5990149f844031f534e65f2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
Date: Sun, 25 Jun 2023 13:38:21 +0200
Subject: [PATCH] org-src--contents-for-write-back: simplify the case of LaTeX
 fragments

* lisp/org-src.el (org-src--contents-for-write-back): Extract special
case logic for LaTeX fragments.
(org-edit-latex-fragment): Trim the contents to be inserted in the
original buffer.
---
 lisp/org-src.el | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index f15ba8e99..5c272c7f5 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -466,7 +466,6 @@ Assume point is in the corresponding edit buffer."
 		  org-src--content-indentation
 		0
 	(use-tabs? (and (> org-src--tab-width 0) t))
-(preserve-fl (eq org-src--source-type 'latex-fragment))
 	(source-tab-width org-src--tab-width)
 	(contents (org-with-wide-buffer
(let ((eol (line-end-position)))
@@ -479,16 +478,13 @@ Assume point is in the corresponding edit buffer."
   ;; 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))
   (insert (org-no-properties (car (cdr contents
   (goto-char (point-min))
-  (when (functionp write-back) (save-excursion (funcall write-back)))
   ;; Add INDENTATION-OFFSET to every line in buffer,
   ;; unless indentation is meant to be preserved.
   (when (> indentation-offset 0)
-	(when preserve-fl (forward-line))
 (while (not (eobp))
 	  (skip-chars-forward " \t")
   (when (or (not (eolp))   ; not a blank line
@@ -498,6 +494,9 @@ Assume point is in the corresponding edit buffer."
 	  (delete-region (line-beginning-position) (point))
 	  (indent-to (+ i indentation-offset
 	  (forward-line)))
+  ;; Apply WRITE-BACK function on edit buffer contents.
+  (goto-char (point-min))
+  (when (functionp write-back) (save-excursion (funcall write-back)))
   (set-marker marker nil
 
 (defun org-src--edit-element
@@ -1150,7 +1149,14 @@ Throw an error when not at such a table."
(lambda ()
 	 ;; Blank lines break things, replace with a single newline.
 	 (while (re-search-forward "\n[ \t]*\n" nil t) (replace-match "\n"))
-	 ;; If within a table a newline would disrupt the structure,
+ ;; Trim contents.
+	 (goto-char (point-min))
+ (skip-chars-forward " \t")
+	 (delete-region (point-min) (point))
+ (goto-char (point-max))
+	 (skip-chars-backward " \t")
+	 (delete-region (point) (point-max))
+ ;; If within a table a newline would disrupt the structure,
 	 ;; so remove newlines.
 	 (goto-char (point-min))
 	 (when (org-element-lineage context '(table-cell))
-- 
2.41.0

From ab48e9671efdaba6566f406b1df81a441c72252c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
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.
* 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 |  4 ++-
 lisp/org-src.el  | 36 ++---
 testing/lisp/test-org-src.el | 62 +++-
 3 files changed, 60 insertions(+), 42 deletions(-)

diff --git a/lisp/org-macs.el b/lisp/org-macs.el
index 51dbfe118..f42e6b14b 100644
--- a/lisp/org-macs.el
+++ b/lisp/org-macs.el
@@ -485,7 +485,9 @@ line.  Return nil if it fails."
 	  (let ((ind (progn (skip-chars-forward " \t") (current-column
 	(cond 

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

2023-06-26 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Ihor Radchenko writes:
>> May you please provide an example for such breakage?
>>  From my point of view, alignment is far lesser concern compared to
>> indentation breaking code execution/tangling/other functional parts of Org.
>
> Let's say tab-width is 8 and elisp-mode uses tabs for indentation. In
> the following snippet, arg2 should be indented with a tab.
>
> #+BEGIN_SRC emacs-lisp
> (some-f arg1
>  arg2) ;; arg2 is indented at column 8
> #+END_SRC
>
> If I add two spaces at the beginning of the block. It should visually
> look like this:
>
> #+BEGIN_SRC emacs-lisp
>(some-f arg1
>  arg2) ;; arg2 is indented at column 8
> #+END_SRC

This is not a problem. We can just apply appropriate 'display property
in `org-src-font-lock-fontify-block', manually replacing the tab with
appropriate number of spaces (as in the origin buffer).

> Calling =indent-to= would rather put the two spaces after the tab
> character.

Then, we should not use `indent-to' and instead put spaces manually.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-26 Thread Sébastien Miquel



Ihor Radchenko writes:

May you please provide an example for such breakage?
 From my point of view, alignment is far lesser concern compared to
indentation breaking code execution/tangling/other functional parts of Org.


Let's say tab-width is 8 and elisp-mode uses tabs for indentation. In
the following snippet, arg2 should be indented with a tab.

#+BEGIN_SRC emacs-lisp
(some-f arg1
arg2) ;; arg2 is indented at column 8
#+END_SRC

If I add two spaces at the beginning of the block. It should visually
look like this:

#+BEGIN_SRC emacs-lisp
  (some-f arg1
arg2) ;; arg2 is indented at column 8
#+END_SRC

Calling =indent-to= would rather put the two spaces after the tab
character. But then we lose this idea of clean separation between the
org indentation and the native indentation, and we will need to
convert the indent of every line on subsequent edits (org -> elisp),
as well as for tangling, or code execution. If we have to do this, we
might as well just respect the org buffer =indent-tabs-mode=, and redo
every indentation with each conversion. This could possibly be costly
when tangling a large buffer.

The visual breakage would be much more common than those more serious
issues, which had gone unnoticed so far.

--
Sébastien Miquel



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

2023-06-26 Thread Ihor Radchenko
Sébastien Miquel  writes:

> On second thought, I don't think that's a good idea. I did not
> understand how tab characters worked: they do not have a fixed width,
> but align to the next tab column. This means that if we add a couple
> of spaces in front of a tab indented piece of code, we can break
> vertical alignment in the org buffer, which I think is pretty bad.

May you please provide an example for such breakage?
>From my point of view, alignment is far lesser concern compared to
indentation breaking code execution/tangling/other functional parts of Org.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-26 Thread Sébastien Miquel



Sébastien Miquel writes:

Should we use native buffer's value of =indent-tabs-mode= to set
=use-tabs?= ? I think this trivial change should work.


It doesn't seem quite that easy. If we want to add 4 columns to a tab
indented line (and tab width is 8), we can either call =indent-to= to
indent with a tab and 4 spaces, or add the spaces at the beginning.
With the second option, we risk breaking vertical alignement in the
org buffer. With the first option, on a subsequent edit, the current
=org-do-remove-indentation= will break the tab character into 4
spaces, making the indentation 8 spaces instead of a tab. You need to
have =org-do-remove-indentation= re-indent the whole line, with the
correct value of =indent-tabs-mode=.

--
Sébastien Miquel



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

2023-06-26 Thread Sébastien Miquel

Hi Ihor,

Ihor Radchenko writes:

...But I guess
what you propose amounts to
...

You are right.


On second thought, I don't think that's a good idea. I did not
understand how tab characters worked: they do not have a fixed width,
but align to the next tab column. This means that if we add a couple
of spaces in front of a tab indented piece of code, we can break
vertical alignment in the org buffer, which I think is pretty bad.

Should we use native buffer's value of =indent-tabs-mode= to set
=use-tabs?= ? I think this trivial change should work.

--
Sébastien Miquel



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

2023-06-25 Thread Ihor Radchenko
Ihor Radchenko  writes:

> Side note: It looks like `org-edit-special' docstring does not mention
> all the cases it considers. In particular, latex fragments are not
> mentioned.

Fixed on main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f8b0b2bab

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-20 Thread Sébastien Miquel



Ihor Radchenko writes:

...But I guess
what you propose amounts to
...

You are right.


I'll try to write a patch, this week-end.


I was not aware of how we treated inline src blocks, but I don't think
so. LaTeX fragments, in particular $$…$$ fragments, can have
significant (for the user) newlines.

May you provide an example?
AFAIK, LaTeX usually treats newlines as whitespace, same with " ".

When I say significant, I don't mean for compilation. When editing an
array of equations for example, one might want to keep one equation
per line in the buffer.

Then, may latex-fragment-specific logic be moved to
`org-edit-latex-fragment'? It already provides WRITE-BACK in
the `org-src--edit-element' call. We may as well take care about
clearing up indentation of the first line and other things there.


Looks like it could, yes : call the WRITE-BACK after indentation. This
doesn't seem to conflict for other org elements.

--
Sébastien Miquel



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

2023-06-20 Thread Ihor Radchenko
Sébastien Miquel  writes:

>> 1. When `org-src-preserve-indentation' is in effect, remove the common
>> `org-src-preserve-indentation' + #+begin indentation from the body.
>
> You've mixed up =org-src-preserve-indentation= and
> =org-edit-src-content-indentation= so I may misunderstand.

You are right. I was referring to the value of
`org-edit-src-content-indentation' when talking about the indentation
offset.

> ...But I guess
> what you propose amounts to
> ...

You are right.

>>> I was not aware of how we treated inline src blocks, but I don't think
>>> so. LaTeX fragments, in particular $$…$$ fragments, can have
>>> significant (for the user) newlines.
>>
>> May you provide an example?
>> AFAIK, LaTeX usually treats newlines as whitespace, same with " ".
>
> When I say significant, I don't mean for compilation. When editing an
> array of equations for example, one might want to keep one equation
> per line in the buffer.

Then, may latex-fragment-specific logic be moved to
`org-edit-latex-fragment'? It already provides WRITE-BACK in
the `org-src--edit-element' call. We may as well take care about
clearing up indentation of the first line and other things there.

Side note: It looks like `org-edit-special' docstring does not mention
all the cases it considers. In particular, latex fragments are not
mentioned.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-19 Thread Sébastien Miquel



Ihor Radchenko writes:


What about the following approach:

When converting from org-src buffer back to Org,

1. We do not touch the original indentation, except minimal common
indentation of the whole src code, respecting the src mode value of
`indent-tabs-mode'.
2. Minimal common indentation is treated according to
`org-src-preserve-indentation'.
3. `org-src-preserve-indentation', when in effect, will add extra
indentation of #+begin indentation + `org-src-preserve-indentation',
now honouring `indent-tabs-mode' in Org buffer.

When converting from Org to org-src buffer,

1. When `org-src-preserve-indentation' is in effect, remove the common
`org-src-preserve-indentation' + #+begin indentation from the body.


You've mixed up =org-src-preserve-indentation= and
=org-edit-src-content-indentation= so I may misunderstand. But I guess
what you propose amounts to

 1. When =org-src-preserve-indentation= is =t=, do not touch
indentation one way or the other (same as now).
 2. Otherwise, do what we do now, but for the common indentation in
the org buffer, use the org value of =indent-tabs-mode=, and for
the rest of the indentation, use the native value of
=indent-tabs-mode=. In this case, instead of trying to read this
value, we might as well just blindly add the common indentation,
to every non empty line.




... "- Item $abc\n  efg$"

Shouldn't newlines be removed completely before editing the body here?
Just like what we do for inline src blocks. See `org-babel--normalize-body'.


I was not aware of how we treated inline src blocks, but I don't think
so. LaTeX fragments, in particular $$…$$ fragments, can have
significant (for the user) newlines.


May you provide an example?
AFAIK, LaTeX usually treats newlines as whitespace, same with " ".


When I say significant, I don't mean for compilation. When editing an
array of equations for example, one might want to keep one equation
per line in the buffer.


--
Sébastien Miquel



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

2023-06-19 Thread Ihor Radchenko
Sébastien Miquel  writes:

> Ihor Radchenko writes:
>> I feel that we will be getting various edge cases like the original
>> report and like the one I made up above if we keep trying to convert
>> tabs/spaces. Just retaining the original code indentation will be much
>> more robust, IMHO.
>
> Python code being broken with the default configuration is
> problematic, and fixing that seems worth the downsides (the
> indentation in an org file will now partially depend on the
> =indent-tabs-mode= settings of other modes, which cannot be set using
> buffer local variables).

What about the following approach:

When converting from org-src buffer back to Org,

1. We do not touch the original indentation, except minimal common
   indentation of the whole src code, respecting the src mode value of
   `indent-tabs-mode'.
2. Minimal common indentation is treated according to
   `org-src-preserve-indentation'.
3. `org-src-preserve-indentation', when in effect, will add extra
   indentation of #+begin indentation + `org-src-preserve-indentation',
   now honouring `indent-tabs-mode' in Org buffer.

When converting from Org to org-src buffer,

1. When `org-src-preserve-indentation' is in effect, remove the common
   `org-src-preserve-indentation' + #+begin indentation from the body.

>>> ... "- Item $abc\n  efg$"
>> Shouldn't newlines be removed completely before editing the body here?
>> Just like what we do for inline src blocks. See `org-babel--normalize-body'.
>
> I was not aware of how we treated inline src blocks, but I don't think
> so. LaTeX fragments, in particular $$…$$ fragments, can have
> significant (for the user) newlines.

May you provide an example?
AFAIK, LaTeX usually treats newlines as whitespace, same with " ".

>>>2. Renaming of =preserve-blank-line=, for clarity.
>> My concern is for `newline-and-indent'. Current line is _previous_  line
>> in such scenario, not the newly inserted line.
>
> The way =newline-and-indent= works, I think, is that a newline is
> inserted, then =org-indent-line= is called, which preindents the line
> to the common org indentation, then calls =TAB= in a native edit
> buffer that does the rest of the indentation. The "current" line I
> refer to in the code is the new line (the "current" line is the one
> from which the native edit was called).

I think I stumbled upon related problem in my testing, but can no longer
reproduce. Your explanation indeed makes sense for my example.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-19 Thread Sébastien Miquel



Ihor Radchenko writes:

I feel that we will be getting various edge cases like the original
report and like the one I made up above if we keep trying to convert
tabs/spaces. Just retaining the original code indentation will be much
more robust, IMHO.


Python code being broken with the default configuration is
problematic, and fixing that seems worth the downsides (the
indentation in an org file will now partially depend on the
=indent-tabs-mode= settings of other modes, which cannot be set using
buffer local variables).


   - =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.
... "- Item $abc\n  efg$"

Shouldn't newlines be removed completely before editing the body here?
Just like what we do for inline src blocks. See `org-babel--normalize-body'.


I was not aware of how we treated inline src blocks, but I don't think
so. LaTeX fragments, in particular $$…$$ fragments, can have
significant (for the user) newlines.


Here are three patches attached.
   1. Two tests : about editing LaTeX fragments, and preserving empty
  lines.

LGTM.


   2. Renaming of =preserve-blank-line=, for clarity.

My concern is for `newline-and-indent'. Current line is _previous_  line
in such scenario, not the newly inserted line.


The way =newline-and-indent= works, I think, is that a newline is
inserted, then =org-indent-line= is called, which preindents the line
to the common org indentation, then calls =TAB= in a native edit
buffer that does the rest of the indentation. The "current" line I
refer to in the code is the new line (the "current" line is the one
from which the native edit was called).

--
Sébastien Miquel



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

2023-06-18 Thread Ihor Radchenko
Sébastien Miquel  writes:

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

You are right.

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

Sure. Or passing -i switch. But that's wrong, IMHO.
Org mode files should be portable. We are aiming for universal
Emacs-independent markup. The value of `indent-tabs-mode' must not
affect Org markup - third party apps should be able to read src block
code without knowledge about Emacs minor modes being active when the Org
file is created. And Org files should not be broken for other users with
different settings for indent-tabs-mode.

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

I think that mixed indentation in the Org buffer is fine. In particular,
in verbatim-type environments.

Let me demonstrate another related problem that manifests more clearly:

1. emacs -Q /tmp/bug.org (new file)
2. C-c C-, s python :results output 
3. M-x whitespace-mode 
4. if True:
5. if True:
6. print("Yes")
7. Observe tab in front of print("Yes")
8. M-: (require 'ob-python) 
9. C-c C-c yes 
10. Observe python error

  File "", line 3
print("yes")
TabError: inconsistent use of tabs and spaces in indentation
[ Babel evaluation exited with code 1 ]

We can, indeed, fix this problem as well by re-indenting the src code
body. But that is (1) awkward; (2) will not work if there happen to be a
language that actually uses mixed indentation - by converting tabs to
spaces we are ultimately using the original info about src body.

I feel that we will be getting various edge cases like the original
report and like the one I made up above if we keep trying to convert
tabs/spaces. Just retaining the original code indentation will be much
more robust, IMHO.

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

> ... "- Item $abc\n  efg$"

Shouldn't newlines be removed completely before editing the body here?
Just like what we do for inline src blocks. See `org-babel--normalize-body'.

> Here are three patches attached.
>   1. Two tests : about editing LaTeX fragments, and preserving empty
>  lines.

LGTM.

>   2. Renaming of =preserve-blank-line=, for clarity.

My concern is for `newline-and-indent'. Current line is _previous_ line
in such scenario, not the newly inserted line.

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

Let's discuss indent-tabs-mode first. I do not yet have a clear
picture about the best Org behaviour.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



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

2023-06-17 Thread Sébastien Miquel

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.1632663...@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 MiquelFrom 9613a54d20883e56301f987f5495b962f3763cad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Miquel?= 
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
+#+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
+
+  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\n  efg$"
+ (org-edit-special)
+ (org-edit-src-exit)
+ (buffer-string)
+
 (ert-deftest test-org-src/footnote-references ()
   "Test editing footnote references."
   ;; 

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

2023-06-14 Thread Ihor Radchenko
wolf  writes:

> Reproduction steps:
> 1. Have org-mode and yaml-mode installed.
> 2. C-x C-f /tmp/x.org RET
> 3. Type in:
>
> #+begin_src yaml
>   a:
> b:
>   c:
>   d:
> #+end_src
>
> After each line, press RET to go to the next one.  RET is bound to org-return.
> Notice that when you press RET after `c:', emacs will insert a TAB character,
> instead of expected 8 spaces.

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

The attached diff will fix the issue, but git log reveals that the whole
thing is fragile - we adjusted code in this area multiple times with
a number of tricky special cases:

https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=e1c49af76
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=70e65a202
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=ee0fd1ec3
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=70e65a202
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=857ae366b
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=91236a3db

https://orgmode.org/list/0f961d3b-a4ef-fbbf-ab16-7ff1af8b2...@gmail.com
https://orgmode.org/list/5DCBAF63-0E88-44AC-B892-1260F37E7E00@manicmind.earth

CCing Bastien and Sébastien, the authors of the previous commits in this
area.

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.

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 317682844..876310867 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -499,12 +499,9 @@ (defun org-src--contents-for-write-back (write-back-buf)
 	(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
- preserve-blank-line))
-	(let ((i (current-column)))
-	  (delete-region (line-beginning-position) (point))
-	  (indent-to (+ i indentation-offset
+	  (let ((i (current-column)))
+	(delete-region (line-beginning-position) (point))
+	(indent-to (+ i indentation-offset)))
 	  (forward-line)))
   (set-marker marker nil
 

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 


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

2023-06-14 Thread wolf
Hello,

when editing a source block of yaml type, the indentation does not work properly
(tabs are used instead of spaces).  I am not sure if org or yaml-mode are the
cause, however since RET is bound to org-return, I am reporting it here first.

I did test it inside a container with nothing but emacs, so no user
configuration is involved at all.

Reproduction steps:
1. Have org-mode and yaml-mode installed.
2. C-x C-f /tmp/x.org RET
3. Type in:

#+begin_src yaml
  a:
b:
  c:
d:
#+end_src

After each line, press RET to go to the next one.  RET is bound to org-return.
Notice that when you press RET after `c:', emacs will insert a TAB character,
instead of expected 8 spaces.

When editing a yaml document in yaml-mode (either a separate document, or using
the C-c '), the indentation works properly.



yaml-mode: version 0.0.15 (but I did test the latest from melpa with same 
result)

Emacs  : GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.37, 
cairo version 1.16.0)
Package: Org mode version 9.6.6 ( @ /home/wolf/.emacs.d/elpa/org-9.6.6/)

current state:
==
(setq
 org-link-elisp-confirm-function 'yes-or-no-p
 org-bibtex-headline-format-function #[257 "\300^A\236A\207" [:title] 3 
"\n\n(fn ENTRY)"]
 org-persist-after-read-hook '(org-element--cache-persist-after-read)
 org-export-before-parsing-hook '(org-attach-expand-links)
 org-cycle-tab-first-hook '(org-babel-hide-result-toggle-maybe 
org-babel-header-arg-expand)
 org-archive-hook '(org-attach-archive-delete-maybe)
 org-cycle-hook '(org-cycle-hide-archived-subtrees org-cycle-show-empty-lines
  org-cycle-optimize-window-after-visibility-change
  org-cycle-display-inline-images)
 org-persist-before-read-hook '(org-element--cache-persist-before-read)
 org-mode-hook '(#[0 "\300\301\302\303\304$\207"
   [add-hook change-major-mode-hook org-fold-show-all append 
local] 5]
 #[0 "\300\301\302\303\304$\207"
   [add-hook change-major-mode-hook org-babel-show-result-all 
append local] 5]
 org-babel-result-hide-spec org-babel-hide-all-hashes)
 org-confirm-shell-link-function 'yes-or-no-p
 outline-isearch-open-invisible-function 'outline-isearch-open-invisible
 org-agenda-before-write-hook '(org-agenda-add-entry-text)
 org-src-mode-hook '(org-src-babel-configure-edit-buffer 
org-src-mode-configure-edit-buffer)
 org-confirm-elisp-link-function 'yes-or-no-p
 org-speed-command-hook '(org-speed-command-activate 
org-babel-speed-command-activate)
 org-fold-core-isearch-open-function 'org-fold-core--isearch-reveal
 org-persist-before-write-hook '(org-element--cache-persist-before-write)
 org-tab-first-hook '(org-babel-hide-result-toggle-maybe 
org-babel-header-arg-expand)
 org-link-shell-confirm-function 'yes-or-no-p
 org-babel-pre-tangle-hook '(save-buffer)
 org-agenda-loop-over-headlines-in-active-region nil
 org-occur-hook '(org-first-headline-recenter)
 org-metadown-hook '(org-babel-pop-to-session-maybe)
 org-link-parameters '(("attachment" :follow org-attach-follow :complete
org-attach-complete-link)
   ("id" :follow org-id-open)
   ("eww" :follow org-eww-open :store org-eww-store-link)
   ("rmail" :follow org-rmail-open :store 
org-rmail-store-link)
   ("mhe" :follow org-mhe-open :store org-mhe-store-link)
   ("irc" :follow org-irc-visit :store org-irc-store-link 
:export
org-irc-export)
   ("info" :follow org-info-open :export org-info-export 
:store
org-info-store-link :insert-description
org-info-description-as-command)
   ("gnus" :follow org-gnus-open :store org-gnus-store-link)
   ("docview" :follow org-docview-open :export 
org-docview-export :store
org-docview-store-link)
   ("bibtex" :follow org-bibtex-open :store 
org-bibtex-store-link)
   ("bbdb" :follow org-bbdb-open :export org-bbdb-export 
:complete
org-bbdb-complete-link :store org-bbdb-store-link)
   ("w3m" :store org-w3m-store-link)
   ("doi" :follow org-link-doi-open :export 
org-link-doi-export)
   ("file+sys") ("file+emacs") ("shell" :follow 
org-link--open-shell)
   ("news" :follow
#[514 "\301\300\302^DQ^B\"\207" ["news" browse-url ":"] 
6
  "\n\n(fn URL ARG)"]
)
   ("mailto" :follow
#[514 "\301\300\302^DQ^B\"\207" ["mailto" browse-url 
":"] 6
  "\n\n(fn URL ARG)"]
)
   ("https" :follow
#[514 "\301\300\302^DQ^B\"\207" ["https" browse-url 
":"]