unofficial mirror of emacs-orgmode@gnu.org
 help / color / mirror / code / Atom feed
* [PATCH] Replace call in org-columns of org-map-entries with org-scan-tags
@ 2021-05-03 13:03 Nick Savage
  2021-05-03 14:00 ` Bastien
  0 siblings, 1 reply; 2+ messages in thread
From: Nick Savage @ 2021-05-03 13:03 UTC (permalink / raw)
  To: emacs-orgmode

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

Hello all,

I was trying to track down the source of a bug encountered when I was 
submitting my last patch about org-columns. I'm going to walk you 
through my thought process and summarize at the end. As part of my 
experimenting, I did the following:

1. emacs -Q
2. open a new buffer at "/tmp/test.org"
3. Add a headline (something like "* test"), and run org-columns only be 
greeted by an unexpected error: "Non-existent agenda file 
/tmp/test.org.  [R]emove from list or [A]bort?"

It's clear that /tmp/test.org is not part of the agenda since it's a 
brand new file, but I was surprised that org-columns cares at all. 
Pressing "R" to remove it from the list tells the user that it wasn't in 
the agenda list (as you'd expect) and then continues to display columns 
as expected, which made me wonder why it is doing that at all. This 
issue only appears though when /tmp/test.org is not saved to disk, which 
looks like expected behaviour as per the following.

I looked into what's going on, and it relates to org-map-entries, which 
builds a list of what is in the buffer. org-map-entries calls 
org-agenda-prepare-buffers. This checks whether the given files are 
actual files or not and opens them up if they are. This makes sense I 
believe from an org-agenda perspective since it has to make sure all of 
the files are open in order to return a list of the headlines, but for 
org-columns it does not. org-columns is an interactive function that is 
only run on the current buffer, so the buffer should always be open.

org-map-entries has a lot of different paths in it based on the 
parameters, but essentially the call from org-columns does two things: 
1) call org-agenda-prepare-buffers and 2) sets "res" to the output of 
org-scan-tags. I believe that the former is unnecessary and that the 
latter is the only necessary code to run in this instance.

To summarize: I've attached a patch to this that makes a change to this. 
It replaces org-map-entries with a call instead to org-scan-tags. This 
fixes the bug I noted above, and I believe (if I'm using the profiler 
correct), makes org-columns a bit faster since it's not calling a bunch 
of functions that doesn't matter to its output. As far as I can tell, my 
patch works as expected with no obvious behaviour changes. All of the 
tests continue to pass as well. I've tested it on my own files and it is 
working as expected as well.

I'd appreciate any feedback on this, or if there are edge cases that the 
removed code breaks that I haven't been able to identify.

Nick


[-- Attachment #2: 0001-org-colview.el-org-columns-Replace-org-map-entries-w.patch --]
[-- Type: text/x-patch, Size: 2450 bytes --]

From f8425ef6fe0ae59c9c4df0b71f3cb61c7bc7e975 Mon Sep 17 00:00:00 2001
From: Nicholas Savage <nick@nicksavage.ca>
Date: Mon, 3 May 2021 08:49:11 -0400
Subject: [PATCH] org-colview.el (org-columns): Replace org-map-entries with
 org-scan-tags

* lisp/org-colview.el (org-columns): Replace call to `org-map-entries'
to build cache with `org-scan-tags'.

Simplifies `org-columns' to call `org-scan-tags' instead of
`org-map-entries'. This is to fix a bug where an unexpected non-existent
agenda file error was thrown if the buffer that `org-columns' was called
on was not yet saved to disk. This also simplifies `org-columns' and
prevents unnecessary agenda preparation functions from running.
---
 lisp/org-colview.el | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/lisp/org-colview.el b/lisp/org-colview.el
index 517bcdf1c..ac3fd9326 100644
--- a/lisp/org-colview.el
+++ b/lisp/org-colview.el
@@ -836,12 +836,11 @@ Also sets `org-columns-top-level-marker' to the new position."
 (defun org-columns (&optional global columns-fmt-string)
   "Turn on column view on an Org mode file.
 
-Column view applies to the whole buffer if point is before the
-first headline.  Otherwise, it applies to the first ancestor
-setting \"COLUMNS\" property.  If there is none, it defaults to
-the current headline.  With a `\\[universal-argument]' prefix \
-argument, turn on column
-view for the whole buffer unconditionally.
+Column view applies to the whole buffer if point is before the first
+headline.  Otherwise, it applies to the first ancestor setting
+\"COLUMNS\" property.  If there is none, it defaults to the current
+headline.  With a `\\[universal-argument]' prefix \ argument, GLOBAL,
+turn on column view for the whole buffer unconditionally.
 
 When COLUMNS-FMT-STRING is non-nil, use it as the column format."
   (interactive "P")
@@ -867,9 +866,8 @@ When COLUMNS-FMT-STRING is non-nil, use it as the column format."
 	(let ((cache
 	       ;; Collect contents of columns ahead of time so as to
 	       ;; compute their maximum width.
-	       (org-map-entries
-		(lambda () (cons (point) (org-columns--collect-values)))
-		nil nil (and org-columns-skip-archived-trees 'archive))))
+               (org-scan-tags
+		(lambda () (cons (point) (org-columns--collect-values))) t org--matcher-tags-todo-only)))
 	  (when cache
 	    (org-columns--set-widths cache)
 	    (org-columns--display-here-title)
-- 
2.20.1


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

* Re: [PATCH] Replace call in org-columns of org-map-entries with org-scan-tags
  2021-05-03 13:03 [PATCH] Replace call in org-columns of org-map-entries with org-scan-tags Nick Savage
@ 2021-05-03 14:00 ` Bastien
  0 siblings, 0 replies; 2+ messages in thread
From: Bastien @ 2021-05-03 14:00 UTC (permalink / raw)
  To: Nick Savage; +Cc: emacs-orgmode

Hi Nick,

Nick Savage <nick@nicksavage.ca> writes:

> I was trying to track down the source of a bug encountered when I was
> submitting my last patch about org-columns. I'm going to walk you
> through my thought process and summarize at the end. As part of my
> experimenting, I did the following:
>
> 1. emacs -Q
> 2. open a new buffer at "/tmp/test.org"
> 3. Add a headline (something like "* test"), and run org-columns only
> be greeted by an unexpected error: "Non-existent agenda file
> /tmp/test.org.  [R]emove from list or [A]bort?"

thanks for catching this, the careful analysis and the patch.

Applied to the maint branch.

Thanks,

-- 
 Bastien


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

end of thread, other threads:[~2021-05-03 14:08 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03 13:03 [PATCH] Replace call in org-columns of org-map-entries with org-scan-tags Nick Savage
2021-05-03 14:00 ` Bastien

unofficial mirror of emacs-orgmode@gnu.org

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/orgmode/0 orgmode/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 orgmode orgmode/ https://yhetil.org/orgmode \
		emacs-orgmode@gnu.org
	public-inbox-index orgmode

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.orgmode
	nntp://news.gmane.io/gmane.emacs.orgmode


code repositories for project(s) associated with this inbox:

	orgmode.git.git (no URL configured)

AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git