emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* One broken property drawer prevents setting of any property
@ 2014-05-06  5:07 Eric Abrahamsen
  2014-05-23  5:56 ` Bastien
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Abrahamsen @ 2014-05-06  5:07 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Nicolas Goaziou

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

Looks like gmane's down for a bit, but presumably this will
eventually go through.

As I mentioned in the last message, if any property drawer in an org
file is malformed, it makes it impossible to set properties on any other
heading in the file. This is because, before the property is set, the
file is scanned for other valid property keys, and when the scanning
process hits the malformed drawer, it errors out.

The attached file, opened from emacs -Q, is enough to cause the error.
Trying to set a property on the second heading gives:

Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
  org-buffer-property-keys(nil t t)
  org-read-property-name()
  org-set-property(nil nil)
  call-interactively(org-set-property nil nil)

`org-buffer-property-keys' contains a call to `org-get-property-block',
which returns nil on a broken property drawer, and leads to the type
argument above.

By passing the FORCE argument to `org-get-property-block', the broken
block ends up getting silently repaired, and everything works as normal.
I'm not sure, however, that silently repairing things without the user's
knowledge is the right thing to do...

Eric


[-- Attachment #2: test.org --]
[-- Type: application/vnd.lotus-organizer, Size: 59 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: One broken property drawer prevents setting of any property
  2014-05-06  5:07 One broken property drawer prevents setting of any property Eric Abrahamsen
@ 2014-05-23  5:56 ` Bastien
  2014-05-23  8:55   ` Eric Abrahamsen
  0 siblings, 1 reply; 4+ messages in thread
From: Bastien @ 2014-05-23  5:56 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: Nicolas Goaziou, emacs-orgmode

Hi Eric,

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> By passing the FORCE argument to `org-get-property-block', the broken
> block ends up getting silently repaired, and everything works as normal.

Can you show this as a patch?

> I'm not sure, however, that silently repairing things without the user's
> knowledge is the right thing to do...

We can warn the user with a temporary message, or ask him for
confirmation.  Or provide a helper command to repair drawers and
advertize it instead of throwing an error.

I'll look into this after you send me the patch.

Thanks,

-- 
 Bastien

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: One broken property drawer prevents setting of any property
  2014-05-23  5:56 ` Bastien
@ 2014-05-23  8:55   ` Eric Abrahamsen
  2014-05-23  9:07     ` Bastien
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Abrahamsen @ 2014-05-23  8:55 UTC (permalink / raw)
  To: emacs-orgmode; +Cc: Bastien, Nicolas Goaziou

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

Bastien <bzg@gnu.org> writes:

> Hi Eric,
>
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>> By passing the FORCE argument to `org-get-property-block', the broken
>> block ends up getting silently repaired, and everything works as normal.
>
> Can you show this as a patch?
>
>> I'm not sure, however, that silently repairing things without the user's
>> knowledge is the right thing to do...
>
> We can warn the user with a temporary message, or ask him for
> confirmation.  Or provide a helper command to repair drawers and
> advertize it instead of throwing an error.
>
> I'll look into this after you send me the patch.

The more I thought about it, the more it seemed the silent repair is a
bad idea: it's not really repair, it's just sticking a new :END: in, and
will very likely result in cruft in the buffer. This patch wraps the
scan in catch/throw and asks users if they want to repair broken
drawers. Since the y-or-no-p stops at the broken drawer, it should be
useful for manual fixing.

E


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Warn-users-of-malformed-property-drawers.patch --]
[-- Type: text/x-diff, Size: 1443 bytes --]

From 3995f4816ba400f5c7645f84b5dcf7e84698d604 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Fri, 23 May 2014 16:52:58 +0800
Subject: [PATCH] Warn users of malformed property drawers

org.el (org-buffer-property-keys): When scanning the buffer for valid
property keys, give users a chance to repair any malformed property
drawers.
---
 lisp/org.el | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index b16515e..a3f0cba 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -15712,12 +15712,17 @@ formats in the current buffer."
 	(widen)
 	(goto-char (point-min))
 	(while (re-search-forward org-property-start-re nil t)
-	  (setq range (org-get-property-block))
-	  (goto-char (car range))
-	  (while (re-search-forward org-property-re
-		  (cdr range) t)
-	    (add-to-list 'rtn (org-match-string-no-properties 2)))
-	  (outline-next-heading))))
+	  (catch 'cont
+	    (setq range (or (org-get-property-block)
+			    (if (y-or-n-p
+				 (format "Malformed drawer at %d, repair?" (point)))
+				(org-get-property-block nil nil t)
+				(throw 'cont nil))))
+	    (goto-char (car range))
+	    (while (re-search-forward org-property-re
+				      (cdr range) t)
+	      (add-to-list 'rtn (org-match-string-no-properties 2)))
+	    (outline-next-heading)))))
 
     (when include-specials
       (setq rtn (append org-special-properties rtn)))
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: One broken property drawer prevents setting of any property
  2014-05-23  8:55   ` Eric Abrahamsen
@ 2014-05-23  9:07     ` Bastien
  0 siblings, 0 replies; 4+ messages in thread
From: Bastien @ 2014-05-23  9:07 UTC (permalink / raw)
  To: Eric Abrahamsen; +Cc: Nicolas Goaziou, emacs-orgmode

Hi Eric,

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> The more I thought about it, the more it seemed the silent repair is a
> bad idea: it's not really repair, it's just sticking a new :END: in, and
> will very likely result in cruft in the buffer. This patch wraps the
> scan in catch/throw and asks users if they want to repair broken
> drawers. Since the y-or-no-p stops at the broken drawer, it should be
> useful for manual fixing.

Agreed and applied on master, thanks a lot!

-- 
 Bastien

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-05-23  9:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06  5:07 One broken property drawer prevents setting of any property Eric Abrahamsen
2014-05-23  5:56 ` Bastien
2014-05-23  8:55   ` Eric Abrahamsen
2014-05-23  9:07     ` Bastien

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).