[PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Sebastian Miele]

* lisp/org-src.el (org-src--contents-for-write-back): Use the
potentially buffer-local value of `org-edit-src-content-indentation'
from the source buffer instead of that from the editing buffer.
---
 lisp/org-src.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/org-src.el b/lisp/org-src.el
index 9134d5b5d..b7fe4c0fa 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -422,7 +422,8 @@ Assume point is in the corresponding edit buffer."
 	 (if org-src--preserve-indentation 0
 	   (+ (or org-src--block-indentation 0)
 	      (if (memq org-src--source-type '(example-block src-block))
-		  org-edit-src-content-indentation
+		  (with-current-buffer (marker-buffer org-src--beg-marker)
+		    org-edit-src-content-indentation)
 		0))))
 	(use-tabs? (and (> org-src--tab-width 0) t))
 	(source-tab-width org-src--tab-width)
-- 
2.23.0

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Adam Porter]

Hi Sebastian,

Sebastian Miele <sebastian.miele@gmail.com> writes:

> * lisp/org-src.el (org-src--contents-for-write-back): Use the
> potentially buffer-local value of `org-edit-src-content-indentation'
> from the source buffer instead of that from the editing buffer.
> ---
>  lisp/org-src.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/org-src.el b/lisp/org-src.el
> index 9134d5b5d..b7fe4c0fa 100644
> --- a/lisp/org-src.el
> +++ b/lisp/org-src.el
> @@ -422,7 +422,8 @@ Assume point is in the corresponding edit buffer."
>  	 (if org-src--preserve-indentation 0
>  	   (+ (or org-src--block-indentation 0)
>  	      (if (memq org-src--source-type '(example-block src-block))
> -		  org-edit-src-content-indentation
> +		  (with-current-buffer (marker-buffer org-src--beg-marker)
> +		    org-edit-src-content-indentation)
>  		0))))
>  	(use-tabs? (and (> org-src--tab-width 0) t))
>  	(source-tab-width org-src--tab-width)

You might consider using the function buffer-local-value instead of the
macro with-current-buffer.  Not that it matters so much here, but
benchmarking shows that it is much faster when simply accessing the
buffer-local value of a variable.

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Sebastian Miele]

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

Hello Adam,

Adam Porter <adam@alphapapa.net> writes:
> You might consider using the function buffer-local-value instead of the
> macro with-current-buffer.  Not that it matters so much here, but
> benchmarking shows that it is much faster when simply accessing the
> buffer-local value of a variable.

Thank you. Such information is always very welcome.

An updated patch is attached to this mail. I also added an ORG-NEWS
entry.

Best wishes
Sebastian


[-- Attachment #2: 0001-Respect-buffer-local-value-of-org-edit-src-content-i.patch --]
[-- Type: text/plain, Size: 1684 bytes --]

From 34eb8882e09701aa12da40510a24c688f4a5ac20 Mon Sep 17 00:00:00 2001
From: Sebastian Miele <sebastian.miele@gmail.com>
Date: Wed, 9 Oct 2019 01:00:50 +0000
Subject: [PATCH] Respect buffer-local value of
 `org-edit-src-content-indentation'

* lisp/org-src.el (org-src--contents-for-write-back): Use the
potentially buffer-local value of `org-edit-src-content-indentation'
from the source buffer instead of that from the editing buffer.
---
 etc/ORG-NEWS    | 4 ++++
 lisp/org-src.el | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 0e07326cb..b562a0935 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -428,6 +428,10 @@ leave unfolded subtrees unfolded.
 I.e. treat the whole file as if it was a subtree.
 
 *** Respect narrowing when agenda command is restricted to buffer
+*** Respect buffer-local value of ~org-edit-src-content-indentation~
+
+Use the potentially buffer-local value of `org-edit-src-content-indentation'
+from the source buffer instead of that from the editing buffer.
 
 * Version 9.2
 ** Incompatible changes
diff --git a/lisp/org-src.el b/lisp/org-src.el
index 9134d5b5d..99841c211 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -422,7 +422,8 @@ Assume point is in the corresponding edit buffer."
 	 (if org-src--preserve-indentation 0
 	   (+ (or org-src--block-indentation 0)
 	      (if (memq org-src--source-type '(example-block src-block))
-		  org-edit-src-content-indentation
+		  (buffer-local-value 'org-edit-src-content-indentation
+				      (marker-buffer org-src--beg-marker))
 		0))))
 	(use-tabs? (and (> org-src--tab-width 0) t))
 	(source-tab-width org-src--tab-width)
-- 
2.23.0

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Nicolas Goaziou]

Hello,

Sebastian Miele <sebastian.miele@gmail.com> writes:

> Subject: [PATCH] Respect buffer-local value of
>  `org-edit-src-content-indentation'
>
> * lisp/org-src.el (org-src--contents-for-write-back): Use the
> potentially buffer-local value of `org-edit-src-content-indentation'
> from the source buffer instead of that from the editing buffer.

Thank you.

Would you mind explaining your use case?

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Sebastian Miele]

Hi Nicolas,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Sebastian Miele <sebastian.miele@gmail.com> writes:
>
>> Subject: [PATCH] Respect buffer-local value of
>>  `org-edit-src-content-indentation'
>>
>> * lisp/org-src.el (org-src--contents-for-write-back): Use the
>> potentially buffer-local value of `org-edit-src-content-indentation'
>> from the source buffer instead of that from the editing buffer.
>
> Thank you.
>
> Would you mind explaining your use case?

The Org default of org-edit-src-content-indentation is 2. I like that
value and leave it that way. Worg's root .dir-locals sets it to 0
buffer-locally in at least many Worg's Org files. Hence, when I edit an
src block in a Worg file, the value of org-edit-src-content-indentation
in the edit buffer is 2. But the correct value in that case is 0.

Best wishes
Sebastian

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Nicolas Goaziou]

Hello,

Sebastian Miele <sebastian.miele@gmail.com> writes:

> The Org default of org-edit-src-content-indentation is 2. I like that
> value and leave it that way. Worg's root .dir-locals sets it to 0
> buffer-locally in at least many Worg's Org files. Hence, when I edit an
> src block in a Worg file, the value of org-edit-src-content-indentation
> in the edit buffer is 2. But the correct value in that case is 0.

Then I think we should do the same as what is done for, e.g.,
`org-src--preserve-indentation`, i.e. store the initial value from
`org-src--edit-element', and use it in
`org-src--contents-for-write-back'. 

WDYT?

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Sebastian Miele]

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

Hi Nicolas,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Sebastian Miele <sebastian.miele@gmail.com> writes:
>
>> The Org default of org-edit-src-content-indentation is 2. I like that
>> value and leave it that way. Worg's root .dir-locals sets it to 0
>> buffer-locally in at least many Worg's Org files. Hence, when I edit an
>> src block in a Worg file, the value of org-edit-src-content-indentation
>> in the edit buffer is 2. But the correct value in that case is 0.
>
> Then I think we should do the same as what is done for, e.g.,
> `org-src--preserve-indentation`, i.e. store the initial value from
> `org-src--edit-element', and use it in
> `org-src--contents-for-write-back'.
>
> WDYT?

That's perfectly fine with me. An updated patch is attached.

Best wishes
Sebastian

[-- Attachment #2: 0001-Respect-buffer-local-value-of-org-edit-src-content-i.patch --]
[-- Type: text/plain, Size: 3404 bytes --]

From a8575e16e2496b2bdcddaaae2532f510a5b18908 Mon Sep 17 00:00:00 2001
From: Sebastian Miele <sebastian.miele@gmail.com>
Date: Wed, 9 Oct 2019 01:00:50 +0000
Subject: [PATCH] Respect buffer-local value of
 `org-edit-src-content-indentation'

* lisp/org-src.el (org-src--content-indentation): Introduce
permanently buffer-local variable for storing away the potentially
buffer-local value of `org-edit-src-content-indentation' in the source
buffer.

(org-src--contents-for-write-back): Use `org-src--content-indentation'
instead of `org-edit-src-content-indentation' in the edit buffer.

(org-src--edit-element): Set `org-src--content-indentation' in editing
buffer.  For greater clarity and consistency, rename already existing
let-bound variable `ind' to `block-ind'.

* etc/ORG-NEWS: Add entry.
---
 etc/ORG-NEWS    |  4 ++++
 lisp/org-src.el | 15 ++++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 283f32e0c..eab9e021b 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -434,6 +434,10 @@ leave unfolded subtrees unfolded.
 I.e. treat the whole file as if it was a subtree.
 
 *** Respect narrowing when agenda command is restricted to buffer
+*** Respect buffer-local value of ~org-edit-src-content-indentation~
+
+Use the potentially buffer-local value of `org-edit-src-content-indentation'
+from the source buffer instead of that from the editing buffer.
 
 * Version 9.2
 ** Incompatible changes
diff --git a/lisp/org-src.el b/lisp/org-src.el
index 9134d5b5d..418f3a662 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -258,6 +258,9 @@ issued in the language major mode buffer."
 (defvar-local org-src--block-indentation nil)
 (put 'org-src--block-indentation 'permanent-local t)
 
+(defvar-local org-src--content-indentation nil)
+(put 'org-src--content-indentation 'permanent-local t)
+
 (defvar-local org-src--end-marker nil)
 (put 'org-src--end-marker 'permanent-local t)
 
@@ -422,7 +425,7 @@ Assume point is in the corresponding edit buffer."
 	 (if org-src--preserve-indentation 0
 	   (+ (or org-src--block-indentation 0)
 	      (if (memq org-src--source-type '(example-block src-block))
-		  org-edit-src-content-indentation
+		  org-src--content-indentation
 		0))))
 	(use-tabs? (and (> org-src--tab-width 0) t))
 	(source-tab-width org-src--tab-width)
@@ -484,9 +487,10 @@ Leave point in edit buffer."
 	     (source-file-name (buffer-file-name (buffer-base-buffer)))
 	     (source-tab-width (if indent-tabs-mode tab-width 0))
 	     (type (org-element-type datum))
-	     (ind (org-with-wide-buffer
-		   (goto-char (org-element-property :begin datum))
-		   (current-indentation)))
+	     (block-ind (org-with-wide-buffer
+			 (goto-char (org-element-property :begin datum))
+			 (current-indentation)))
+	     (content-ind org-edit-src-content-indentation)
 	     (preserve-ind
 	      (and (memq type '(example-block src-block))
 		   (or (org-element-property :preserve-indent datum)
@@ -529,7 +533,8 @@ Leave point in edit buffer."
 	(setq org-src--end-marker end)
 	(setq org-src--remote remote)
 	(setq org-src--source-type type)
-	(setq org-src--block-indentation ind)
+	(setq org-src--block-indentation block-ind)
+	(setq org-src--content-indentation content-ind)
 	(setq org-src--preserve-indentation preserve-ind)
 	(setq org-src--overlay overlay)
 	(setq org-src--allow-write-back write-back)
-- 
2.24.0

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Nicolas Goaziou]

Hello,

Sebastian Miele <sebastian.miele@gmail.com> writes:

> That's perfectly fine with me. An updated patch is attached.

Applied. I removed the ORG-NEWS entry because it is more a bugfix than
anything else.

Thank you.

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Bastien]

Hi Sebastian,

Sebastian Miele <sebastian.miele@gmail.com> writes:

> * lisp/org-src.el (org-src--contents-for-write-back): Use the
> potentially buffer-local value of `org-edit-src-content-indentation'
> from the source buffer instead of that from the editing buffer.

I'm not sure about I see why this patch would be useful.

IIUC, in case `org-edit-src-content-indentation' is set in the src
block buffer itself, you want Org to honor its value _there_ instead
of the value this variable has in the org buffer (containing the src
block)?

I find it difficult to see a compelling use case, please let me know
if I don't understand correctly.

Thanks,

-- 
 Bastien

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Sebastian Miele]

Hi Bastien,

Bastien <bzg@gnu.org> writes:
>
> Sebastian Miele <sebastian.miele@gmail.com> writes:
>
> > * lisp/org-src.el (org-src--contents-for-write-back): Use the
> > potentially buffer-local value of `org-edit-src-content-indentation'
> > from the source buffer instead of that from the editing buffer.
>
> I'm not sure about I see why this patch would be useful.
>
> IIUC, in case `org-edit-src-content-indentation' is set in the src
> block buffer itself, you want Org to honor its value _there_ instead
> of the value this variable has in the org buffer (containing the src
> block)?

I am quite certain, that the content indentation conceptually and
technically belongs to the buffer containing the src block. Except for
src blocks containing org I see no point at all for setting a content
indentation in an edit buffer for the block. There just is no meaning of
an org src content indentation in e.g. an Elisp buffer.

And for an src buffer containing org the meaning would be the content
intentation for the org in the block, and not for the org containing the
block.

> I find it difficult to see a compelling use case, please let me know
> if I don't understand correctly.

Nicolas already accepted the patch. Currently it is included as commit
3649d95b in the history of master. Before accepting the patch, he too
asked for a use case. Please see

https://lists.gnu.org/archive/html/emacs-orgmode/2019-11/msg00052.html

for the answer.

Best wishes
Sebastian

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Bastien]

Hi Sebastian,

Sebastian Miele <sebastian.miele@gmail.com> writes:

> I am quite certain, that the content indentation conceptually and
> technically belongs to the buffer containing the src block.

Sorry, I think I got confused - let's call buffer-A the one with the 
"#+begin_src ... #+end_src" string and buffer-B the one you get when
C-c ' on this string (the src block).

If "the buffer containing the src block" is buffer A, then we are on
the same line.  Is it the case?  Or am I still confused?

-- 
 Bastien

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Sebastian Miele]

Bastien <bzg@gnu.org> writes:
>
> Sebastian Miele <sebastian.miele@gmail.com> writes:
>
> > I am quite certain, that the content indentation conceptually and
> > technically belongs to the buffer containing the src block.
>
> Sorry, I think I got confused - let's call buffer-A the one with the
> "#+begin_src ... #+end_src" string and buffer-B the one you get when
> C-c ' on this string (the src block).
>
> If "the buffer containing the src block" is buffer A, then we are on
> the same line.  Is it the case?  Or am I still confused?

Yes, we are on the same line.

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Sebastian Miele]

Sebastian Miele <sebastian.miele@gmail.com> writes:

> Bastien <bzg@gnu.org> writes:
> >
> > Sebastian Miele <sebastian.miele@gmail.com> writes:
> >
> > > I am quite certain, that the content indentation conceptually and
> > > technically belongs to the buffer containing the src block.
> >
> > Sorry, I think I got confused - let's call buffer-A the one with the
> > "#+begin_src ... #+end_src" string and buffer-B the one you get when
> > C-c ' on this string (the src block).
> >
> > If "the buffer containing the src block" is buffer A, then we are on
> > the same line.  Is it the case?  Or am I still confused?
>
> Yes, we are on the same line.

Part of me feels that I should find a better explanation and spare you
the trouble of figuring out what I mean. But there seems to be no
(significantly) better explanation I can come up with without going to
greater lengths and especially investing considerable time for working
out very clear terminology for the situation.

I currently feel a tension between not wanting you to have to spend
considerable time understanding what I wrote and not wanting myself to
spend time to try to explain it better.

However, if it is necessary, I will take the time.

Please try having it in the back of your mind for some days, maybe even
without actively spending much time on trying to understand it. Then
tell me if struck or not.

All the best
Sebastian

Re: [PATCH] Respect buffer-local value of `org-edit-src-content-indentation'

[Bastien]

Hi Sebastian,

Sebastian Miele <sebastian.miele@gmail.com> writes:

> Part of me feels that I should find a better explanation and spare you
> the trouble of figuring out what I mean. But there seems to be no
> (significantly) better explanation I can come up with without going to
> greater lengths and especially investing considerable time for working
> out very clear terminology for the situation.

You explained things very clearly, it is just that I was somehow tired
and those things are complex to explain (and figure out) by nature.

> I currently feel a tension between not wanting you to have to spend
> considerable time understanding what I wrote and not wanting myself to
> spend time to try to explain it better.

Well, it's always a tradeoff between the time the one reporting the
bug can spend producing a sensible report and the time the bug fixer
can spend understanding and fixing it.

And each side tends to underestimate the time it takes to the other
side... the only big asymmetry here is that there are more reporters
than fixers :)

> However, if it is necessary, I will take the time.

Thanks for this...

> Please try having it in the back of your mind for some days, maybe even
> without actively spending much time on trying to understand it. Then
> tell me if struck or not.

... I'll remember it for next times -- but in this case, the lack of
understanding was because of me.

Best,

-- 
 Bastien