[BUG, PATCH] org-indent-mode not correctly deactivated

[BUG, PATCH] org-indent-mode not correctly deactivated

[Bastien]

With a file like

,----
| #+STARTUP: indent
| 
| * Heading
| ** Subheading
`----

org-mode will correctly use org-indent-mode when displaying it.

Now edit the file to deactivate org-indent:

,----
| #+STARTUP: noindent
| 
| * Heading
| ** Subheading
`----

Go to #+STARTUP and hit C-c C-c : org-indent-mode will not be
activated, but it will not be properly deactivated, leaving a
confusing whitespace before indented headlines.

The attached patch fixes the problem, but it is wrong, because it
"actively" deactivates org-indent-mode each time a buffer is not
using org-indent-mode, in sessions where org-indent-mode has been
used at least once.

I could not come up with a better fix -- Nicolas, Carsten, do
you have an idea on how to fix this?

Thanks!

-- 
 Bastien

Re: [BUG, PATCH] org-indent-mode not correctly deactivated

[Nicolas Goaziou]

Hello,

Bastien <bzg@gnu.org> writes:

> With a file like
>
> ,----
> | #+STARTUP: indent
> | 
> | * Heading
> | ** Subheading
> `----
>
> org-mode will correctly use org-indent-mode when displaying it.
>
> Now edit the file to deactivate org-indent:
>
> ,----
> | #+STARTUP: noindent
> | 
> | * Heading
> | ** Subheading
> `----
>
> Go to #+STARTUP and hit C-c C-c : org-indent-mode will not be
> activated, but it will not be properly deactivated, leaving a
> confusing whitespace before indented headlines.
>
> The attached patch fixes the problem, but it is wrong, because it
> "actively" deactivates org-indent-mode each time a buffer is not
> using org-indent-mode, in sessions where org-indent-mode has been
> used at least once.

There is no attached patch in your mail. Though, I don't understand why
you think your approach is wrong. In `org-mode' mode definition, we can
change:

  (when org-startup-indented (require 'org-indent) (org-indent-mode 1))

into:

  (cond (org-startup-indented (require 'org-indent) (org-indent-mode 1))
        ((org-bound-and-true-p org-indent-mode) (org-indent-mode -1)))

It's probably similar to what you wrote in your patch.


Regards,

-- 
Nicolas Goaziou

Re: [BUG, PATCH] org-indent-mode not correctly deactivated

[Bastien]

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

Hi Nicolas,

Nicolas Goaziou <n.goaziou@gmail.com> writes:

> There is no attached patch in your mail. Though, I don't understand why
> you think your approach is wrong. In `org-mode' mode definition, we can
> change:
>
>   (when org-startup-indented (require 'org-indent) (org-indent-mode 1))
>
> into:
>
>   (cond (org-startup-indented (require 'org-indent) (org-indent-mode 1))
>         ((org-bound-and-true-p org-indent-mode) (org-indent-mode -1)))

This will not work, because (org-bound-and-true-p org-indent-mode)
returns nil at the time `org-mode' is called.

> It's probably similar to what you wrote in your patch.

Here is my patch -- quite similar, except that it will disable
`org-indent-mode' for every restart once it has been activated
once, which is wrong, or at least to heavy-handed.

Maybe we can simply tell users to deactivate `org-indent-mode'
manually, but this feels wrong too.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix-org-indent.patch --]
[-- Type: text/x-diff, Size: 610 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index ee7965a..f77a0e1 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -5350,7 +5350,8 @@ The following commands are available:
   (modify-syntax-entry ?@ "w")
   (modify-syntax-entry ?\" "\"")
   (if org-startup-truncated (setq truncate-lines t))
-  (when org-startup-indented (require 'org-indent) (org-indent-mode 1))
+  (cond (org-startup-indented (require 'org-indent) (org-indent-mode 1))
+	((fboundp 'org-indent-initialize) (org-indent-mode 0)))
   (org-set-local 'font-lock-unfontify-region-function
 		 'org-unfontify-region)
   ;; Activate before-change-function

[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


-- 
 Bastien

Re: [BUG, PATCH] org-indent-mode not correctly deactivated

[Nicolas Goaziou]

Bastien <bzg@gnu.org> writes:

> Nicolas Goaziou <n.goaziou@gmail.com> writes:
>
>>   (cond (org-startup-indented (require 'org-indent) (org-indent-mode 1))
>>         ((org-bound-and-true-p org-indent-mode) (org-indent-mode -1)))
>
> This will not work, because (org-bound-and-true-p org-indent-mode)
> returns nil at the time `org-mode' is called.

If it returns nil, it means that `org-startup-indented' is nil and
either `org-indent' isn't loaded or `org-indent-mode' is off.

I don't get why it wouldn't work.


Regards,

-- 
Nicolas Goaziou

Re: [BUG, PATCH] org-indent-mode not correctly deactivated

[Bastien]

Nicolas Goaziou <n.goaziou@gmail.com> writes:

> Bastien <bzg@gnu.org> writes:
>
>> Nicolas Goaziou <n.goaziou@gmail.com> writes:
>>
>>>   (cond (org-startup-indented (require 'org-indent) (org-indent-mode 1))
>>>         ((org-bound-and-true-p org-indent-mode) (org-indent-mode -1)))
>>
>> This will not work, because (org-bound-and-true-p org-indent-mode)
>> returns nil at the time `org-mode' is called.
>
> If it returns nil, it means that `org-startup-indented' is nil and
> either `org-indent' isn't loaded or `org-indent-mode' is off.
>
> I don't get why it wouldn't work.

Try to edebug-defun `org-mode' with the recipe I posted and you'll
see: yes, `org-startup-indented' and `org-indent-mode' are off,
but the org-indent properties have not been removed from the before,
as they are by manually turning off with M-x org-indent-mode RET

-- 
 Bastien

Re: [BUG, PATCH] org-indent-mode not correctly deactivated

[Bastien]

Bastien <bzg@gnu.org> writes:

> Try to edebug-defun `org-mode' with the recipe I posted and you'll
> see: yes, `org-startup-indented' and `org-indent-mode' are off,
> but the org-indent properties have not been removed from the before,
                                                               ^^^^^^
                                                               buffer

/me english speaking trying.

-- 
 Bastien

Re: [BUG, PATCH] org-indent-mode not correctly deactivated

[Nicolas Goaziou]

Bastien <bzg@gnu.org> writes:

> Nicolas Goaziou <n.goaziou@gmail.com> writes:
>
>> Bastien <bzg@gnu.org> writes:
>>
>>> Nicolas Goaziou <n.goaziou@gmail.com> writes:
>>>
>>>>   (cond (org-startup-indented (require 'org-indent) (org-indent-mode 1))
>>>>         ((org-bound-and-true-p org-indent-mode) (org-indent-mode -1)))
>>>
>>> This will not work, because (org-bound-and-true-p org-indent-mode)
>>> returns nil at the time `org-mode' is called.
>>
>> If it returns nil, it means that `org-startup-indented' is nil and
>> either `org-indent' isn't loaded or `org-indent-mode' is off.
>>
>> I don't get why it wouldn't work.
>
> Try to edebug-defun `org-mode' with the recipe I posted and you'll
> see: yes, `org-startup-indented' and `org-indent-mode' are off,
> but the org-indent properties have not been removed from the before,
> as they are by manually turning off with M-x org-indent-mode RET

I see, thank you. But I still don't understand where, and when,
`org-indent-mode' is reset.

Anyway, we can also patch `org-mode-restart', from

  (defun org-mode-restart ()
    (interactive)
    (funcall major-mode)
    (hack-local-variables)
    (message "%s restarted" major-mode)) 

to,

  (defun org-mode-restart ()
    (interactive)
    (let ((indent-status (org-bound-and-true-p org-indent-mode)))
      (funcall major-mode)
      (hack-local-variables)
      (when (and indent-status (not (org-bound-and-true-p org-indent-mode)))
        (org-indent-mode -1)))
    (message "%s restarted" major-mode))

Still not pretty, but a bit better, IMO.

Also, there's a dangling (defvar org-indent-mode nil) in org.el that
looks suspicious. I don't think we need it if we use
`org-bound-and-true-p' whenever we need to check for `org-indent-mode'
value.


Regards,

-- 
Nicolas Goaziou

Re: [BUG, PATCH] org-indent-mode not correctly deactivated

[Bastien]

Nicolas Goaziou <n.goaziou@gmail.com> writes:

>   (defun org-mode-restart ()
>     (interactive)
>     (let ((indent-status (org-bound-and-true-p org-indent-mode)))
>       (funcall major-mode)
>       (hack-local-variables)
>       (when (and indent-status (not (org-bound-and-true-p org-indent-mode)))
>         (org-indent-mode -1)))
>     (message "%s restarted" major-mode))
>
> Still not pretty, but a bit better, IMO.

Definitely better, I applied this.

> Also, there's a dangling (defvar org-indent-mode nil) in org.el that
> looks suspicious. I don't think we need it if we use
> `org-bound-and-true-p' whenever we need to check for `org-indent-mode'
> value.

Indeed, I removed this.

Thanks!

-- 
 Bastien