org-hide-leading-stars-before-indent-mode

org-hide-leading-stars-before-indent-mode

[D]

Hi all,

I have come across this rather strange variable in org-indent, and was
wondering if someone could add a bit of documentation to clarify what it
actually does.

Currently, the variable org-hide-leading-stars-before-indent-mode is
only documented as "used locally".  However, upon closer inspection, the
first thing I noticed was that it is actually declared using "defvar"
instead of "defvar-local", which in turn would render the docstring
redundant as it would automatically append the message "this variable
becomes buffer local when set".  Is this intentional?  Looking at the
code it seems to be a throwaway variable remembering the initial value
of org-hide-leading-stars, restoring it buffer-locally when indent-mode
is turned off.  This leaves two questions for me:

1) Wouldn't it be clearer to use defvar-local for
org-hide-leading-stars-before-indent-mode and replace the docstring with
something like "Holds the original value of `org-hide-leading-stars'
before Org indent."

2) Considering that org-hide-leading-stars is global by default,
wouldn't it be even better to dispose of the temp variable entirely?
One could either use (kill-local-variable 'org-hide-leading-stars) or
(setq-local org-hide-leading-stars (default-value
'org-hide-leading-stars)) for the same effect.

Cheers,
D.

Re: org-hide-leading-stars-before-indent-mode

[Bastien]

Hi,

D <d.williams@posteo.net> writes:

> 1) Wouldn't it be clearer to use defvar-local for
> org-hide-leading-stars-before-indent-mode and replace the docstring with
> something like "Holds the original value of `org-hide-leading-stars'
> before Org indent."

Yes, patch welcome.

> 2) Considering that org-hide-leading-stars is global by default,
> wouldn't it be even better to dispose of the temp variable entirely?
> One could either use (kill-local-variable 'org-hide-leading-stars) or
> (setq-local org-hide-leading-stars (default-value
> 'org-hide-leading-stars)) for the same effect.

IIUC, it would be "and", not "or": first use

  (setq-local org-hide-leading-stars (default-value 'org-hide-leading-stars))

to set org-hide-leading-stars to a temporary value *and*

  (kill-local-variable 'org-hide-leading-stars)

to restore the global value.

If you can make something that works, please send a patch.

Thanks!

-- 
 Bastien

[PATCH] Re: org-hide-eading-stars-before-indent-mode

[D]

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

Hi!

On 19.02.20 22:42, Bastien wrote:
> IIUC, it would be "and", not "or": first use
> 
>   (setq-local org-hide-leading-stars (default-value 'org-hide-leading-stars))
> 
> to set org-hide-leading-stars to a temporary value *and*
> 
>   (kill-local-variable 'org-hide-leading-stars)
> 
> to restore the global value.

Not quite, both are valid ways to revert hide-leading-stars to it's
original value, with the second (which I picked for the patch) having
the advantage to properly get rid off the buffer-local property again.
The mode itself calls for setting the variable locally to `t' instead of
the original value.

I attached the patch, which I hope is the correct format.  I'm still a
bit new to this, my apologies if I cause any unnecessary work.

Regards,
D.

[-- Attachment #2: 0001-org-indent.el-Deprecate-org-hide-leading-stars-befor.patch --]
[-- Type: text/x-patch, Size: 2578 bytes --]

From d3390eb442bccef476fa2514d4078b9f8e9d978a Mon Sep 17 00:00:00 2001
From: "D. Williams" <d.williams@posteo.net>
Date: Wed, 19 Feb 2020 23:50:48 +0100
Subject: [PATCH] org-indent.el: Deprecate
 `org-hide-leading-stars-before-indent-mode'

* lisp/org-indent.el (org-indent-mode): Make `org-hide-leading-stars'
buffer local and revert it back to it's global value when exiting the
mode without storing it's global value manually.
(org-hide-leading-stars-before-indent-mode): Remove declaration.

This commit implements my suggestion from the mailing list
(https://lists.gnu.org/archive/html/emacs-orgmode/2020-02/msg00759.html)
to rely on Emacs' native mechanisms for restoring a buffer-local
variable to it's global default value instead of storing the global
value in a temporary variable (in this case:
`org-hide-leading-stars-before-indent-mode').

TINYCHANGE
---
 lisp/org-indent.el | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/lisp/org-indent.el b/lisp/org-indent.el
index c136a75bd..73b077965 100644
--- a/lisp/org-indent.el
+++ b/lisp/org-indent.el
@@ -71,8 +71,6 @@ Delay used when the buffer to initialize isn't current.")
 (defvar org-indent--initial-marker nil
   "Position of initialization before interrupt.
 This is used locally in each buffer being initialized.")
-(defvar org-hide-leading-stars-before-indent-mode nil
-  "Used locally.")
 (defvar org-indent-modified-headline-flag nil
   "Non-nil means the last deletion operated on a headline.
 It is modified by `org-indent-notify-modified-headline'.")
@@ -183,8 +181,6 @@ during idle time."
       (or (eq org-adapt-indentation 'headline-data)
 	  (setq-local org-adapt-indentation nil)))
     (when org-indent-mode-turns-on-hiding-stars
-      (setq-local org-hide-leading-stars-before-indent-mode
-		  org-hide-leading-stars)
       (setq-local org-hide-leading-stars t))
     (org-indent--compute-prefixes)
     (if (boundp 'filter-buffer-substring-functions)
@@ -216,9 +212,8 @@ during idle time."
 	  (delq (current-buffer) org-indent-agentized-buffers))
     (when (markerp org-indent--initial-marker)
       (set-marker org-indent--initial-marker nil))
-    (when (boundp 'org-hide-leading-stars-before-indent-mode)
-      (setq-local org-hide-leading-stars
-		  org-hide-leading-stars-before-indent-mode))
+    (when (local-variable-p 'org-hide-leading-stars)
+      (kill-local-variable 'org-hide-leading-stars))
     (if (boundp 'filter-buffer-substring-functions)
 	(remove-hook 'filter-buffer-substring-functions
 		     (lambda (fun start end delete)
-- 
2.24.1

Re: [PATCH] Re: org-hide-eading-stars-before-indent-mode

[Bastien]

Hi,

D <d.williams@posteo.net> writes:

> Not quite, both are valid ways to revert hide-leading-stars to it's
> original value, with the second (which I picked for the patch) having
> the advantage to properly get rid off the buffer-local property again.
> The mode itself calls for setting the variable locally to `t' instead of
> the original value.

Yes, you're right.

> I attached the patch, which I hope is the correct format.  I'm still a
> bit new to this, my apologies if I cause any unnecessary work.

It's perfect, applied to master, thanks!

-- 
 Bastien