Re: [O] [PATCH] Fix capture to make it save the point location
Bastien: did you mean the reply on the Capture Abort thread? This is a different bug, unrelated to that the Capture Abort one. In this case capture doesn't abort, but returns to a different place in buffer than the one it was invoked from. Thanks, Alex On Fri, May 23, 2014 at 5:30 AM, Bastien b...@gnu.org wrote: Hi Alex, Alex Kosorukoff a...@3form.com writes: sorry, I accidentally sent my previous patch. This is the one that belongs here. See my other reply for this bug and let me know if it's fixed. Thanks, -- Bastien
Re: [O] [PATCH] Fix capture to make it save the point location
Bastien: yes, I can reproduce the bug using capmove.el with the latest master. It is less salient in the small file that capmove makes, but very obvious in big files where you get to a completely different place after capture finished. Description of the steps: 1. open the same org file in two frames 2. make sure the positions in those frames are different 3. try to capture something from one of those frames 4. after capture finished, the positions in those frames become the same, i.e. the frame where capture was invoked copied the position from another frame and lost its context that was there before capture. Best, Alex On Fri, May 23, 2014 at 8:39 AM, Bastien b...@altern.org wrote: Hi Alex, Alex Kosorukoff a...@3form.com writes: did you mean the reply on the Capture Abort thread? This is a different bug, unrelated to that the Capture Abort one. In this case capture doesn't abort, but returns to a different place in buffer than the one it was invoked from. Indeed, sorry for the confusion. The thing is: I can't reproduce the problem, even when following the capmove.el instructions. Can you reproduce the bug using latest maint or master branch? If so, a simple description of the steps will be easier than a file like capmove.el. Thanks, -- Bastien
Re: [O] [PATCH] Fix: Capture abort: (error: The mark is not set now, so there is no region)
On Fri, May 23, 2014 at 5:03 AM, Bastien b...@gnu.org wrote: Hi Alex, Alex Kosorukoff a...@3form.com writes: After I replaced my patch and merged Bastien's fix, I started seeing the error though less frequently than before. It didn't occur in the template I posted, but I started seeing it again in another template. (w org-protocol tag entry (file ~/org/bookmarks.org) * %:description %(org-set-tags)\n %i\n\n %:link\n%? :prepend t :empty-lines-after 1 :clock-in t :clock-resume t) If I may ask, why using %(org-set-tags) instead of %^g or %^G? I am using org-set-tags to avoid autocompletion, both %^g and %^G take too much time because my org files have many tagged items. Capture should be fast to be effective. I switched back to my initial patch that was checking if the mark was set before trying to access the region. This worked: the errors disappeared. I think the right fix is to exclude `mark-active' from the local variable that are imported through `org-capture-steal-local-variables'. I installed such a fix in maint, please update Org and let me know if this works for you. Excluding mark-active will work, the result will be the same as after my patch, except performance will not be the same. Excluding variable requires filtering the list of variables which takes O(n) whereas my patch takes O(1). Mark-active is nil before org-capture-steal-local-variables because this is a new buffer. It seems in this case setting it back to nil is faster than trying to preserve its original value nil. Thanks, -- Bastien
Re: [O] [PATCH] Fix: Capture abort: (error: The mark is not set now, so there is no region)
In fact, there is some performance issue. The steal function copies a lot of variables as I can tell. Do you know where those variables are used? I replaced the steal function with an advice like this (defadvice org-capture-steal-local-variables (around do-not-steal activate)) My capture became very fast after that and I didn't notice any adverse effects so far (using this for more than a week). The only reason I didn't propose a patch like this is that I am still testing it for possible regressions. On Fri, May 23, 2014 at 10:28 AM, Bastien b...@gnu.org wrote: Hi Alex, Alex Kosorukoff a...@3form.com writes: Excluding mark-active will work, the result will be the same as after my patch, except performance will not be the same. Excluding variable requires filtering the list of variables which takes O(n) whereas my patch takes O(1). Mark-active is nil before org-capture-steal-local-variables because this is a new buffer. It seems in this case setting it back to nil is faster than trying to preserve its original value nil. I see what you mean but there is no performance issue here and not copying the value of mark-active is cleaner than setting it back to nil -- we never want to copy the value of the mark at all. -- Bastien
Re: [O] [PATCH] Fix: Capture abort: (error: The mark is not set now, so there is no region)
After I replaced my patch and merged Bastien's fix, I started seeing the error though less frequently than before. It didn't occur in the template I posted, but I started seeing it again in another template. (w org-protocol tag entry (file ~/org/bookmarks.org) * %:description %(org-set-tags)\n %i\n\n %:link\n%? :prepend t :empty-lines-after 1 :clock-in t :clock-resume t) I switched back to my initial patch that was checking if the mark was set before trying to access the region. This worked: the errors disappeared. However, I couldn't understand why mark would be unset in the first place. It turns out that this is a result of the function org-capture-steal-local-variables that is copying mark-active variable from another buffer. This results in an inconsistent state where mark is not set and yet the variable mark-active is set. Emacs functions region-active-p and use-region-p expect the state to be consistent and fail with this error that they won't produce under normal circumstances. Here is the minimal patch that fixes the root cause of this problem: From 3d84403964dec1ac55810883e4e8a812c3ff94fc Mon Sep 17 00:00:00 2001 From: Alex Kosorukoff a...@3form.com Date: Thu, 8 May 2014 20:27:59 -0700 Subject: [PATCH] org-capture: better fix for error The mark is not set now, ... --- lisp/org-capture.el |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/lisp/org-capture.el b/lisp/org-capture.el index d57b9e0..3c62b1d 100644 --- a/lisp/org-capture.el +++ b/lisp/org-capture.el @@ -1590,6 +1590,7 @@ The template may still contain \%?\ for cursor positioning. (goto-char (point-min)) (org-capture-steal-local-variables buffer) (setq buffer-file-name nil) + (setq mark-active nil) ;; %[] Insert contents of a file. (goto-char (point-min)) -- 1.7.0.4 On Tue, Apr 29, 2014 at 4:05 AM, Bastien b...@gnu.org wrote: Hi Alex, Alex Kosorukoff a...@3form.com writes: I noticed a regression in the capture functionality after upgrading org. Capture fails with error in subj fixed, thanks, -- Bastien
[O] [PATCH] Fix capture to make it save the point location
Hello:
this is another small patch to org-capture.el to make sure that after
completion it returns to the same place from where it was invoked. This way
users won't loose track of where they were before capturing something. The
minimal setup to reproduce the case where capture fails to return to the
place of its invocation is attached.
Best,
Alex
From ac50a5300e35d7abd5f50317069b2a795fde4ad8 Mon Sep 17 00:00:00 2001
From: Alex Kosorukoff a...@3form.com
Date: Mon, 17 Mar 2014 12:56:09 -0700
Subject: [PATCH] fix org-capture error The mark is not set now, so there is no region
---
lisp/org.el |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lisp/org.el b/lisp/org.el
index dc4f2cc..bc5a69e 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -14611,7 +14611,7 @@ When JUST-ALIGN is non-nil, only align tags.
When JUST-ALIGN is 'ignore-column, align tags without trying to set
the column by ignoring invisible text.
(interactive P)
- (if (and (org-region-active-p) org-loop-over-headlines-in-active-region)
+ (if (and (mark t) (org-region-active-p) org-loop-over-headlines-in-active-region)
(let ((cl (if (eq org-loop-over-headlines-in-active-region 'start-level)
'region-start-level 'region))
org-loop-over-headlines-in-active-region)
--
1.7.0.4
;; capmove.el org-mode capture moving the point in a buffer
;; $ emacs -Q -l capfail.el
(unless window-system
(insert This test needs window-system, exiting...)
(sleep-for 3)
(kill-emacs))
(setq inhibit-splash-screen t)
(add-to-list 'load-path ~/.emacs.d/org/lisp)
(require 'org)
(setq org-capture-templates
'((t Todo entry (file todo.org) * TODO %^{Title}\n %?)))
(define-key global-map (kbd C-c c) 'org-capture)
(switch-to-buffer test.org)
(insert we start here and do some editing\n) (goto-char 0)
(select-frame (make-frame))
(goto-char (point-max))
(dotimes (n 5)
(insert (format %d\n n))
(sit-for 1))
(insert now we invoke capture here)
(sit-for 1)
(setq last-kbd-macro [?\C-c ?c ?t ?t ?e ?s ?t return return])
(sleep-for 3) (kmacro-call-macro nil)
(insert note that capture had already moved the point\n)
(insert to the top of the file in this buffer\n\n)
(sit-for 3)
(insert so we end up in a different place when we finish\n)
(insert even if we abort it with C-c C-k)
(sit-for 3)
(setq last-kbd-macro [?\C-c ?\C-k])
(sleep-for 3) (kmacro-call-macro nil)
(provide 'capmove)
Re: [O] [PATCH] Fix capture to make it save the point location
sorry, I accidentally sent my previous patch. This is the one that belongs here. On Thu, May 1, 2014 at 7:00 PM, Alex Kosorukoff a...@3form.com wrote: Hello: this is another small patch to org-capture.el to make sure that after completion it returns to the same place from where it was invoked. This way users won't loose track of where they were before capturing something. The minimal setup to reproduce the case where capture fails to return to the place of its invocation is attached. Best, Alex From cf97dd81aa94510e5dcd5be478b515c732cd93d4 Mon Sep 17 00:00:00 2001 From: Alex Kosorukoff a...@3form.com Date: Thu, 1 May 2014 18:50:43 -0700 Subject: [PATCH] org-capture: fix org-capture to make it save the point position * lisp/org-capture.el (org-capture-fill template) can change the point position in the buffer where capture was invoked, so user may not return to the same place after capture completion --- lisp/org-capture.el |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/lisp/org-capture.el b/lisp/org-capture.el index c053640..1e3ae5b 100644 --- a/lisp/org-capture.el +++ b/lisp/org-capture.el @@ -584,7 +584,9 @@ of the day at point (if any) or the current HH:MM time. (org-current-time))) (org-capture-set-target-location) (condition-case error - (org-capture-put :template (org-capture-fill-template)) + (org-capture-put :template + (save-excursion + (org-capture-fill-template))) ((error quit) (if (get-buffer *Capture*) (kill-buffer *Capture*)) (error Capture abort: %s error))) -- 1.7.0.4
[O] [PATCH] Fix: Capture abort: (error: The mark is not set now, so there is no region)
I noticed a regression in the capture functionality after upgrading org. Capture fails with error in subj Here is a simple config to reproduce the problem and a patch that fixes it. emacs -q -l capfail.el Best, Alex From ac50a5300e35d7abd5f50317069b2a795fde4ad8 Mon Sep 17 00:00:00 2001 From: Alex Kosorukoff a...@3form.com Date: Mon, 17 Mar 2014 12:56:09 -0700 Subject: [PATCH] fix org-capture error The mark is not set now, so there is no region --- lisp/org.el |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lisp/org.el b/lisp/org.el index dc4f2cc..bc5a69e 100644 --- a/lisp/org.el +++ b/lisp/org.el @@ -14611,7 +14611,7 @@ When JUST-ALIGN is non-nil, only align tags. When JUST-ALIGN is 'ignore-column, align tags without trying to set the column by ignoring invisible text. (interactive P) - (if (and (org-region-active-p) org-loop-over-headlines-in-active-region) + (if (and (mark t) (org-region-active-p) org-loop-over-headlines-in-active-region) (let ((cl (if (eq org-loop-over-headlines-in-active-region 'start-level) 'region-start-level 'region)) org-loop-over-headlines-in-active-region) -- 1.7.0.4 ;; capfail.el org-mode capture failure when region is active ;; $ emacs -q -l capfail.el (setq inhibit-splash-screen t) (add-to-list 'load-path ~/.emacs.d/org/lisp) (require 'org) (setq org-capture-templates '((t Todo entry (file test.org) * TODO Test %^g\n %?))) (define-key global-map (kbd C-c c) 'org-capture) (find-file test.org) (insert Select some text to make a region, then try C-c c t\ntest\n Emacs 23.1.1/23.3.1/24.1/24.2 Org-mode version 8.2.6 result:\n Capture abort: (error: The mark is not set now, so there is no region)\n) (provide 'capfail)
