Bug in org-log-into-drawer

Bug in org-log-into-drawer

[Erik Hetzner]

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

Hi,

Current the org-log-into-drawer function does not honor the value of
the LOG_INTO_DRAWER property if the property has the value nil. For
example, if the org-log-into-drawer variable is set to t, but we have
the file:

  * Foo
    :PROPERTIES:
    :LOG_INTO_DRAWER: nil
    :END:
  ** TODO Bar

org-mode will log changes to the TODO entry into a drawer. The
attached patch fixes the issue, allowing a nil value of the
LOG_INTO_DRAWER property to override a t value of the
org-log-into-drawer variable.

best, Erik


[-- Attachment #2: org-log-into-drawer-prop.diff --]
[-- Type: text/plain, Size: 635 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index 4e79125..2aa70bd 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2519,9 +2519,10 @@ a subtree."
   "Return the value of `org-log-into-drawer', but let properties overrule.
 If the current entry has or inherits a LOG_INTO_DRAWER property, it will be
 used instead of the default value."
-  (let ((p (org-entry-get nil "LOG_INTO_DRAWER" 'inherit)))
+  (let ((p (org-entry-get nil "LOG_INTO_DRAWER" 'inherit t)))
     (cond
-     ((or (not p) (equal p "nil")) org-log-into-drawer)
+     ((not p) org-log-into-drawer)
+     ((equal p "nil") nil)
      ((equal p "t") "LOGBOOK")
      (t p))))
 

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

Sent from my free software system <http://fsf.org/>.

Re: Bug in org-log-into-drawer

[Bastien]

Hi Erik,

thanks for the patch, I just applied it.

Best,

-- 
 Bastien

Re: Bug in org-log-into-drawer

[Sebastien Vauban]

Hello Erik,

Erik Hetzner wrote:
> Current the org-log-into-drawer function does not honor the value of
> the LOG_INTO_DRAWER property if the property has the value nil. For
> example, if the org-log-into-drawer variable is set to t, but we have
> the file:
>
>   * Foo
>     :PROPERTIES:
>     :LOG_INTO_DRAWER: nil
>     :END:
>   ** TODO Bar
>
> org-mode will log changes to the TODO entry into a drawer.

I would have said that this does make sense, as the property LOG_INTO_DRAWER
is not inherited by the TODO entry, does it?

See http://orgmode.org/manual/Property-inheritance.html for the few Org
properties for which inheritance is hard-coded.

> The attached patch fixes the issue, allowing a nil value of the
> LOG_INTO_DRAWER property to override a t value of the org-log-into-drawer
> variable.

Best regards,
  Seb

-- 
Sebastien Vauban

Re: Bug in org-log-into-drawer

[Erik Hetzner]

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

At Sat, 27 Oct 2012 11:00:40 +0200,
Sebastien Vauban wrote:
> 
> Hello Erik,
> 
> I would have said that this does make sense, as the property LOG_INTO_DRAWER
> is not inherited by the TODO entry, does it?
> 
> See http://orgmode.org/manual/Property-inheritance.html for the few Org
> properties for which inheritance is hard-coded.
> 
> > The attached patch fixes the issue, allowing a nil value of the
> > LOG_INTO_DRAWER property to override a t value of the org-log-into-drawer
> > variable.

Hi Seb,

The function org-log-into-drawer called the function org-entry-get
with the inherit argument before I got there. (Maybe it needs to be
added to that list?) This issue had to do with the difference between
the string nil (as supplied in the property) and the value nil
(meaning no property was found).

best, Erik

[-- Attachment #2: Type: text/plain, Size: 53 bytes --]

Sent from my free software system <http://fsf.org/>.

Re: Bug in org-log-into-drawer

[Bastien]

Hi Erik and Sébastien,

Erik Hetzner <egh@e6h.org> writes:

> The function org-log-into-drawer called the function org-entry-get
> with the inherit argument before I got there. (Maybe it needs to be
> added to that list?)

I don't have time to look deeper in this issue.  Can one of you have a
closer look at the manual and see if there is any inconsistency?

Thanks!

-- 
 Bastien

Re: Bug in org-log-into-drawer

[Erik Hetzner]

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

At Mon, 29 Oct 2012 06:41:39 +0100,
Bastien wrote:
> 
> Hi Erik and Sébastien,
> 
> Erik Hetzner <egh@e6h.org> writes:
> 
> > The function org-log-into-drawer called the function org-entry-get
> > with the inherit argument before I got there. (Maybe it needs to be
> > added to that list?)
> 
> I don't have time to look deeper in this issue.  Can one of you have a
> closer look at the manual and see if there is any inconsistency?

Hi Bastien,

org mode manual says:

  You can also overrule the setting of this variable for a subtree by
  setting a `LOG_INTO_DRAWER' property.

This sounds to me that LOG_INTO_DRAWER inheritance is intended. Even
if it is not, I would argue that LOG_INTO_DRAWER *should* apply to
subtrees, because otherwise one would have to set that property on
every TODO item.

best, Erik

[-- Attachment #2: Type: text/plain, Size: 53 bytes --]

Sent from my free software system <http://fsf.org/>.

Re: Bug in org-log-into-drawer

[Sebastien Vauban]

Hello Erik and Bastien,

Erik Hetzner wrote:
> Bastien wrote:
>> Erik Hetzner <egh-r1v5srsr4wc@public.gmane.org> writes:
>> 
>>> The function org-log-into-drawer called the function org-entry-get with
>>> the inherit argument before I got there. (Maybe it needs to be added to
>>> that list?)
>> 
>> I don't have time to look deeper in this issue. Can one of you have a
>> closer look at the manual and see if there is any inconsistency?
>
> Hi Bastien,
>
> org mode manual says:
>
>   You can also overrule the setting of this variable for a subtree by
>   setting a `LOG_INTO_DRAWER' property.
>
> This sounds to me that LOG_INTO_DRAWER inheritance is intended. Even if it
> is not, I would argue that LOG_INTO_DRAWER *should* apply to subtrees,
> because otherwise one would have to set that property on every TODO item.

I share your point...

But do we all agree that your original example had to fail when
LOG_INTO_DRAWER is not inherited?[1]

Or do I miss some point?

Seb

[1] If yes, is your patch still OK?

-- 
Sebastien Vauban

Re: Bug in org-log-into-drawer

[Erik Hetzner]

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

At Mon, 29 Oct 2012 17:07:23 +0100,
Sebastien Vauban wrote:
> 
> Hello Erik and Bastien,
>
> […]
>
> I share your point...
> 
> But do we all agree that your original example had to fail when
> LOG_INTO_DRAWER is not inherited?[1]
> 
> Or do I miss some point?
> 
> Seb

Hi Sebastien,

Yes, I had a bad example. But the function org-log-into-drawer would
not work (before my patch) on the following example either:

  * TODO Foo
    :PROPERTIES:
    :LOG_INTO_DRAWER: nil
    :END:

The issue was with the distinction between the value nil and string
"nil". org-entry-get (without an additional arg) will return the value
nil in the above example, which meant that the value of
org-log-into-drawer (t) was not being overidden.

best, Erik


[-- Attachment #2: Type: text/plain, Size: 53 bytes --]

Sent from my free software system <http://fsf.org/>.