emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] Removed unecessary invocations of org-agenda-show.
@ 2010-09-13 16:48 Matt Lundin
  2010-09-14  7:35 ` Carsten Dominik
  2011-05-02  8:17 ` [Accepted] [Orgmode] " Carsten Dominik
  0 siblings, 2 replies; 6+ messages in thread
From: Matt Lundin @ 2010-09-13 16:48 UTC (permalink / raw)
  To: Org Mode


lisp/org-agenda.el (org-agenda-set-tags): Remove org-agenda-show to prevent
disrupting windows and changing point in original buffer.
(org-agenda-set-property): Same
(org-agenda-set-effort): Same
(org-agenda-toggle-archive-tag): Same

When setting a tag in the agenda, org-mode displays the corresponding
entry in the original org buffer by calling org-agenda-show. This has
the unwelcome side-effect of disrupting the current window arrangement
and changing the position of the point in the original buffer. This
behavior is inconsistent with the that of org-agenda-todo, which makes
all its changes "silently."

Here is the offending line (6799) in org-agenda-set-tags:

--8<---------------cut here---------------start------------->8---
(org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
--8<---------------cut here---------------end--------------->8---

The same line occurs in org-agenda-set-property, org-agenda-set-effort,
and org-agenda-toggle-archive tag.
---
 lisp/org-agenda.el |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 32c65db..784ba6a 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -6796,7 +6796,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
   (org-agenda-check-no-diary)
   (if (and (org-region-active-p) (interactive-p))
       (call-interactively 'org-change-tag-in-region)
-    (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
     (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
 			 (org-agenda-error)))
 	   (buffer (marker-buffer hdmarker))
@@ -6825,7 +6824,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
   "Set a property for the current headline."
   (interactive)
   (org-agenda-check-no-diary)
-  (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
   (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
 		       (org-agenda-error)))
 	 (buffer (marker-buffer hdmarker))
@@ -6848,7 +6846,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
   "Set the effort property for the current headline."
   (interactive)
   (org-agenda-check-no-diary)
-  (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
   (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
 		       (org-agenda-error)))
 	 (buffer (marker-buffer hdmarker))
@@ -6872,7 +6869,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
   "Toggle the archive tag for the current entry."
   (interactive)
   (org-agenda-check-no-diary)
-  (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
   (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
                        (org-agenda-error)))
 	 (buffer (marker-buffer hdmarker))
-- 
1.7.2.3

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

* Re: [PATCH] Removed unecessary invocations of org-agenda-show.
  2010-09-13 16:48 [PATCH] Removed unecessary invocations of org-agenda-show Matt Lundin
@ 2010-09-14  7:35 ` Carsten Dominik
  2010-09-16  4:22   ` Matthew Lundin
  2011-05-02  8:17 ` [Accepted] [Orgmode] " Carsten Dominik
  1 sibling, 1 reply; 6+ messages in thread
From: Carsten Dominik @ 2010-09-14  7:35 UTC (permalink / raw)
  To: Matt Lundin; +Cc: org-mode List


On Sep 13, 2010, at 6:48 PM, Matt Lundin wrote:

>
> lisp/org-agenda.el (org-agenda-set-tags): Remove org-agenda-show to  
> prevent
> disrupting windows and changing point in original buffer.
> (org-agenda-set-property): Same
> (org-agenda-set-effort): Same
> (org-agenda-toggle-archive-tag): Same
>
> When setting a tag in the agenda, org-mode displays the corresponding
> entry in the original org buffer by calling org-agenda-show. This has
> the unwelcome side-effect of disrupting the current window arrangement
> and changing the position of the point in the original buffer. This
> behavior is inconsistent with the that of org-agenda-todo, which makes
> all its changes "silently."

I agree, but I am sure I used to have problems with something
which is why this was added.
Have you been running this patch for some time already?
Without any problems like the agenda jumping to a wrong place in a org  
file or so?
That would be great.

- Carsten


>
> Here is the offending line (6799) in org-agenda-set-tags:
>
> --8<---------------cut here---------------start------------->8---
> (org-agenda-show)   ;;; FIXME This is a stupid hack and should not  
> be needed
> --8<---------------cut here---------------end--------------->8---
>
> The same line occurs in org-agenda-set-property, org-agenda-set- 
> effort,
> and org-agenda-toggle-archive tag.
> ---
> lisp/org-agenda.el |    4 ----
> 1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
> index 32c65db..784ba6a 100644
> --- a/lisp/org-agenda.el
> +++ b/lisp/org-agenda.el
> @@ -6796,7 +6796,6 @@ the same tree node, and the headline of the  
> tree node in the Org-mode file."
>   (org-agenda-check-no-diary)
>   (if (and (org-region-active-p) (interactive-p))
>       (call-interactively 'org-change-tag-in-region)
> -    (org-agenda-show)   ;;; FIXME This is a stupid hack and should  
> not be needed
>     (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
> 			 (org-agenda-error)))
> 	   (buffer (marker-buffer hdmarker))
> @@ -6825,7 +6824,6 @@ the same tree node, and the headline of the  
> tree node in the Org-mode file."
>   "Set a property for the current headline."
>   (interactive)
>   (org-agenda-check-no-diary)
> -  (org-agenda-show)   ;;; FIXME This is a stupid hack and should  
> not be needed
>   (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
> 		       (org-agenda-error)))
> 	 (buffer (marker-buffer hdmarker))
> @@ -6848,7 +6846,6 @@ the same tree node, and the headline of the  
> tree node in the Org-mode file."
>   "Set the effort property for the current headline."
>   (interactive)
>   (org-agenda-check-no-diary)
> -  (org-agenda-show)   ;;; FIXME This is a stupid hack and should  
> not be needed
>   (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
> 		       (org-agenda-error)))
> 	 (buffer (marker-buffer hdmarker))
> @@ -6872,7 +6869,6 @@ the same tree node, and the headline of the  
> tree node in the Org-mode file."
>   "Toggle the archive tag for the current entry."
>   (interactive)
>   (org-agenda-check-no-diary)
> -  (org-agenda-show)   ;;; FIXME This is a stupid hack and should  
> not be needed
>   (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
>                        (org-agenda-error)))
> 	 (buffer (marker-buffer hdmarker))
> -- 
> 1.7.2.3
>
>
> _______________________________________________
> Emacs-orgmode mailing list
> Please use `Reply All' to send replies to the list.
> Emacs-orgmode@gnu.org
> http://lists.gnu.org/mailman/listinfo/emacs-orgmode

- Carsten

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

* Re: [PATCH] Removed unecessary invocations of org-agenda-show.
  2010-09-14  7:35 ` Carsten Dominik
@ 2010-09-16  4:22   ` Matthew Lundin
  2010-10-04  4:22     ` Carsten Dominik
  0 siblings, 1 reply; 6+ messages in thread
From: Matthew Lundin @ 2010-09-16  4:22 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: org-mode List

Hi Carsten,

Carsten Dominik <carsten.dominik@gmail.com> writes:

> On Sep 13, 2010, at 6:48 PM, Matt Lundin wrote:
>
>>
>> lisp/org-agenda.el (org-agenda-set-tags): Remove org-agenda-show to
>> prevent
>> disrupting windows and changing point in original buffer.
>> (org-agenda-set-property): Same
>> (org-agenda-set-effort): Same
>> (org-agenda-toggle-archive-tag): Same
>>
>> When setting a tag in the agenda, org-mode displays the corresponding
>> entry in the original org buffer by calling org-agenda-show. This has
>> the unwelcome side-effect of disrupting the current window arrangement
>> and changing the position of the point in the original buffer. This
>> behavior is inconsistent with the that of org-agenda-todo, which makes
>> all its changes "silently."
>
> I agree, but I am sure I used to have problems with something
> which is why this was added.
> Have you been running this patch for some time already?
> Without any problems like the agenda jumping to a wrong place in a org
> file or so?

I haven't yet encountered any deleterious side effects, but I agree that
this patch needs further consideration. I will do some additional
research/testing and return with a report. :)

Best,
Matt

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

* Re: [PATCH] Removed unecessary invocations of org-agenda-show.
  2010-09-16  4:22   ` Matthew Lundin
@ 2010-10-04  4:22     ` Carsten Dominik
  2010-10-05 18:07       ` Matt Lundin
  0 siblings, 1 reply; 6+ messages in thread
From: Carsten Dominik @ 2010-10-04  4:22 UTC (permalink / raw)
  To: Matthew Lundin; +Cc: org-mode List


On Sep 16, 2010, at 6:22 AM, Matthew Lundin wrote:

> Hi Carsten,
>
> Carsten Dominik <carsten.dominik@gmail.com> writes:
>
>> On Sep 13, 2010, at 6:48 PM, Matt Lundin wrote:
>>
>>>
>>> lisp/org-agenda.el (org-agenda-set-tags): Remove org-agenda-show to
>>> prevent
>>> disrupting windows and changing point in original buffer.
>>> (org-agenda-set-property): Same
>>> (org-agenda-set-effort): Same
>>> (org-agenda-toggle-archive-tag): Same
>>>
>>> When setting a tag in the agenda, org-mode displays the  
>>> corresponding
>>> entry in the original org buffer by calling org-agenda-show. This  
>>> has
>>> the unwelcome side-effect of disrupting the current window  
>>> arrangement
>>> and changing the position of the point in the original buffer. This
>>> behavior is inconsistent with the that of org-agenda-todo, which  
>>> makes
>>> all its changes "silently."
>>
>> I agree, but I am sure I used to have problems with something
>> which is why this was added.
>> Have you been running this patch for some time already?
>> Without any problems like the agenda jumping to a wrong place in a  
>> org
>> file or so?
>
> I haven't yet encountered any deleterious side effects, but I agree  
> that
> this patch needs further consideration. I will do some additional
> research/testing and return with a report. :)

Hi Matt,

any new about this patch?

Thanks

- Carsten

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

* Re: [PATCH] Removed unecessary invocations of org-agenda-show.
  2010-10-04  4:22     ` Carsten Dominik
@ 2010-10-05 18:07       ` Matt Lundin
  0 siblings, 0 replies; 6+ messages in thread
From: Matt Lundin @ 2010-10-05 18:07 UTC (permalink / raw)
  To: Carsten Dominik; +Cc: org-mode List

Hi Carsten

Carsten Dominik <carsten.dominik@gmail.com> writes:

> On Sep 16, 2010, at 6:22 AM, Matthew Lundin wrote:
>
>> Hi Carsten,
>>
>> Carsten Dominik <carsten.dominik@gmail.com> writes:
>>
>>> On Sep 13, 2010, at 6:48 PM, Matt Lundin wrote:
>>>
>>>>
>>>> lisp/org-agenda.el (org-agenda-set-tags): Remove org-agenda-show to
>>>> prevent
>>>> disrupting windows and changing point in original buffer.
>>>> (org-agenda-set-property): Same
>>>> (org-agenda-set-effort): Same
>>>> (org-agenda-toggle-archive-tag): Same
>>>>
>>>> When setting a tag in the agenda, org-mode displays the
>>>> corresponding
>>>> entry in the original org buffer by calling org-agenda-show. This
>>>> has
>>>> the unwelcome side-effect of disrupting the current window
>>>> arrangement
>>>> and changing the position of the point in the original buffer. This
>>>> behavior is inconsistent with the that of org-agenda-todo, which
>>>> makes
>>>> all its changes "silently."
>>>
>>> I agree, but I am sure I used to have problems with something
>>> which is why this was added.
>>> Have you been running this patch for some time already?
>>> Without any problems like the agenda jumping to a wrong place in a
>>> org
>>> file or so?
>>
>> I haven't yet encountered any deleterious side effects, but I agree
>> that
>> this patch needs further consideration. I will do some additional
>> research/testing and return with a report. :)
>
> Hi Matt,
>
> any new about this patch?
>

I've looked at the code and can't see anything that org-agenda-show adds
to org-agenda-set-tags, apart from making the original buffer visible.

The function org-agenda-show calls org-agenda-goto, which grabs the
value of org-marker, jumps to the corresponding buffer and position, and
shows the next heading. The function org-agenda-set-tags does exactly
the same things in the background, except that it uses org-hd-marker
instead of org-marker.

As a point of comparison, org-agenda-todo seems to works just fine
without invoking org-agenda-show.

Unfortunately, there haven't been substantial changes to
org-agenda-set-tags since 4.12a (the beginning of the git repository),
so I can't reconstruct why the hack was added. 

I've not run into any troubles with the patch. I've tried moving
headlines around behind the agenda's back, changing multiple lines
before refreshing the agenda buffer, etc., all without issue. That's not
to say there aren't problems lurking out there somewhere; but detecting
them is beyond my abilities. :)

Best,
Matt

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

* [Accepted] [Orgmode] Removed unecessary invocations of org-agenda-show.
  2010-09-13 16:48 [PATCH] Removed unecessary invocations of org-agenda-show Matt Lundin
  2010-09-14  7:35 ` Carsten Dominik
@ 2011-05-02  8:17 ` Carsten Dominik
  1 sibling, 0 replies; 6+ messages in thread
From: Carsten Dominik @ 2011-05-02  8:17 UTC (permalink / raw)
  To: emacs-orgmode

Patch 271 (http://patchwork.newartisans.com/patch/271/) is now "Accepted".

Maintainer comment: none

This relates to the following submission:

http://mid.gmane.org/%3C87zkvly3m3.fsf%40archdesk.localdomain%3E

Here is the original message containing the patch:

> Content-Type: text/plain; charset="utf-8"
> MIME-Version: 1.0
> Content-Transfer-Encoding: 7bit
> Subject: [Orgmode] Removed unecessary invocations of org-agenda-show.
> Date: Mon, 13 Sep 2010 20:48:27 -0000
> From: Matt Lundin <mdl@imapmail.org>
> X-Patchwork-Id: 271
> Message-Id: <87zkvly3m3.fsf@archdesk.localdomain>
> To: Org Mode <emacs-orgmode@gnu.org>
> 
> lisp/org-agenda.el (org-agenda-set-tags): Remove org-agenda-show to prevent
> disrupting windows and changing point in original buffer.
> (org-agenda-set-property): Same
> (org-agenda-set-effort): Same
> (org-agenda-toggle-archive-tag): Same
> 
> When setting a tag in the agenda, org-mode displays the corresponding
> entry in the original org buffer by calling org-agenda-show. This has
> the unwelcome side-effect of disrupting the current window arrangement
> and changing the position of the point in the original buffer. This
> behavior is inconsistent with the that of org-agenda-todo, which makes
> all its changes "silently."
> 
> Here is the offending line (6799) in org-agenda-set-tags:
> 
> --8<---------------cut here---------------start------------->8---
> (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
> --8<---------------cut here---------------end--------------->8---
> 
> The same line occurs in org-agenda-set-property, org-agenda-set-effort,
> and org-agenda-toggle-archive tag.
> 
> ---
> lisp/org-agenda.el |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
> index 32c65db..784ba6a 100644
> --- a/lisp/org-agenda.el
> +++ b/lisp/org-agenda.el
> @@ -6796,7 +6796,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
>    (org-agenda-check-no-diary)
>    (if (and (org-region-active-p) (interactive-p))
>        (call-interactively 'org-change-tag-in-region)
> -    (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
>      (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
>  			 (org-agenda-error)))
>  	   (buffer (marker-buffer hdmarker))
> @@ -6825,7 +6824,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
>    "Set a property for the current headline."
>    (interactive)
>    (org-agenda-check-no-diary)
> -  (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
>    (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
>  		       (org-agenda-error)))
>  	 (buffer (marker-buffer hdmarker))
> @@ -6848,7 +6846,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
>    "Set the effort property for the current headline."
>    (interactive)
>    (org-agenda-check-no-diary)
> -  (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
>    (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
>  		       (org-agenda-error)))
>  	 (buffer (marker-buffer hdmarker))
> @@ -6872,7 +6869,6 @@ the same tree node, and the headline of the tree node in the Org-mode file."
>    "Toggle the archive tag for the current entry."
>    (interactive)
>    (org-agenda-check-no-diary)
> -  (org-agenda-show)   ;;; FIXME This is a stupid hack and should not be needed
>    (let* ((hdmarker (or (org-get-at-bol 'org-hd-marker)
>                         (org-agenda-error)))
>  	 (buffer (marker-buffer hdmarker))
> 

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

end of thread, other threads:[~2011-05-02  8:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-13 16:48 [PATCH] Removed unecessary invocations of org-agenda-show Matt Lundin
2010-09-14  7:35 ` Carsten Dominik
2010-09-16  4:22   ` Matthew Lundin
2010-10-04  4:22     ` Carsten Dominik
2010-10-05 18:07       ` Matt Lundin
2011-05-02  8:17 ` [Accepted] [Orgmode] " Carsten Dominik

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).