Re: [PATCH] Add tags filter parameter to clock table

From: Carsten Dominik <carsten.dominik@gmail.com>
To: Adam Elliott <aelliott@hgn.ca>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] Add tags filter parameter to clock table
Date: Thu, 4 Mar 2010 22:21:23 +0100	[thread overview]
Message-ID: <5FDA7EA8-FBC5-4B7A-9DAD-A4C0D065653D@gmail.com> (raw)
In-Reply-To: <4B838024.8000006@hgn.ca>

Hi Adam,

great work, it seems this patch does what it claims.  It may be slow  
in some cases,
but I don't think this is a problem.

The patch is more than a few lines, to be sure I would like toask you  
to sign the papers with the FSF?  Is that OK?

http://orgmode.org/worg/org-contribute.php#sec-2

- Carsten

On Feb 23, 2010, at 8:13 AM, Adam Elliott wrote:

> I have attached a git patch against master that implements a new
> parameter to clock tables, "tags".  This parameter is a tags-query  
> as a
> string and is used to filter the headlines which are consulted when
> building the clock table.
>
> In my search of the archives to see if this feature already existed, I
> found a reference here:
>  http://article.gmane.org/gmane.emacs.orgmode/17304
> suggesting it was difficult.  The patch is not so large, though, so
> perhaps I am missing something.
>
> My rationale in implementing this feature was to keep track of the
> occasional task item that is not billable, yet still makes sense to
> include in the overall project structure.  Of course I could just  
> avoid
> clocking the task item, or manually delete clock lines before  
> generating
> a report, but this feature reduces the chance for error; no doubt  
> there
> are other workflows enabled with this feature as well.  I don't make
> significant use of tags myself, but I know many do.
>
> In order to maintain a sensible report, headlines that don't match the
> tag filter may be included if they have descendants that do.  Any time
> clocked directly on non-matching headlines, however, is excluded.
>
> Specifying even a simple filter noticeably slows down clock table
> generation for non-toy reports, particularly for clock table reports
> with :step.  If there is no filter, though, there is no degradation in
> performance.
>
> Tag filter syntax is the standard one, as described at:
>  http://orgmode.org/manual/Matching-tags-and-properties.html
> Only tags are considered at the moment, although I suspect querying
> against all properties would be possible (if even slower).
>
> Examples:
>
> * development
>  CLOCK: => 1:00
> *** task 1
>    CLOCK: => 1:00
> *** task 2                                              :must:
> ***** task 2a
>      CLOCK: => 1:00
> ***** task 2b                                           :mustnot:
>      CLOCK: => 1:00
>
> Note I am using an unconventional but legal(ish) clock format for
> brevity.  Clock tables are also pruned to only relevant lines.
>
> [1] #+BEGIN: clocktable
> |   | *Total time* | *4:00* |      |      |
> |---+--------------+--------+------+------|
> | 1 | development  | 4:00   |      |      |
> | 2 | task 1       |        | 1:00 |      |
> | 2 | task 2       |        | 2:00 |      |
> | 3 | task 2a      |        |      | 1:00 |
> | 3 | task 2b      |        |      | 1:00 |
>
> [2] #+BEGIN: clocktable :tags "must"
> |   | *Total time* | *2:00* |      |      |
> |---+--------------+--------+------+------|
> | 1 | development  | 2:00   |      |      |
> | 2 | task 2       |        | 2:00 |      |
> | 3 | task 2a      |        |      | 1:00 |
> | 3 | task 2b      |        |      | 1:00 |
>
> [3] #+BEGIN: clocktable :tags "-mustnot"
> |   | *Total time* | *3:00* |      |      |
> |---+--------------+--------+------+------|
> | 1 | development  | 3:00   |      |      |
> | 2 | task 1       |        | 1:00 |      |
> | 2 | task 2       |        | 1:00 |      |
> | 3 | task 2a      |        |      | 1:00 |
>
> [4] #+BEGIN: clocktable :tags "must-mustnot"
> |   | *Total time* | *1:00* |      |      |
> |---+--------------+--------+------+------|
> | 1 | development  | 1:00   |      |      |
> | 2 | task 2       |        | 1:00 |      |
> | 3 | task 2a      |        |      | 1:00 |
>
> [5] #+BEGIN: clocktable :tags "must+mustnot"
> |   | *Total time* | *1:00* |      |      |
> |---+--------------+--------+------+------|
> | 1 | development  | 1:00   |      |      |
> | 2 | task 2       |        | 1:00 |      |
> | 3 | task 2b      |        |      | 1:00 |
>
> As you can see, in examples 2, 4, and 5, the time clocked on
> "development" itself is being removed.  Example 2 illustrates the  
> effect
> of tag inheritance.
>
> Adam
> From 80fb7b01c58989ed69b5a176784d71fe557fc4c6 Mon Sep 17 00:00:00 2001
> From: Adam Elliott <aelliott@hgn.ca>
> Date: Mon, 22 Feb 2010 00:57:22 -0500
> Subject: [PATCH] Add tags filter parameter to clocktables.
>
> ---
> lisp/org-clock.el |   59 ++++++++++++++++++++++++++++++++++++++++ 
> +-----------
> 1 files changed, 46 insertions(+), 13 deletions(-)
> mode change 100644 => 100755 lisp/org-clock.el
>
> diff --git a/lisp/org-clock.el b/lisp/org-clock.el
> old mode 100644
> new mode 100755
> index e3866be..fb85380
> --- a/lisp/org-clock.el
> +++ b/lisp/org-clock.el
> @@ -1289,10 +1289,13 @@ With prefix arg SELECT, offer recently  
> clocked tasks for selection."
>   "Holds the file total time in minutes, after a call to `org-clock- 
> sum'.")
> (make-variable-buffer-local 'org-clock-file-total-minutes)
>
> -(defun org-clock-sum (&optional tstart tend)
> +(defun org-clock-sum (&optional tstart tend headline-filter)
>   "Sum the times for each subtree.
> Puts the resulting times in minutes as a text property on each  
> headline.
> -TSTART and TEND can mark a time range to be considered."
> +TSTART and TEND can mark a time range to be considered.  HEADLINE- 
> FILTER is a
> +zero-arg function that, if specified, is called for each headline  
> in the time
> +range with point at the headline.  Headlines for which HEADLINE- 
> FILTER returns
> +nil are excluded from the clock summation."
>   (interactive)
>   (let* ((bmp (buffer-modified-p))
> 	 (re (concat "^\\(\\*+\\)[ \t]\\|^[ \t]*"
> @@ -1308,7 +1311,9 @@ TSTART and TEND can mark a time range to be  
> considered."
>     (if (stringp tend) (setq tend (org-time-string-to-seconds tend)))
>     (if (consp tstart) (setq tstart (org-float-time tstart)))
>     (if (consp tend) (setq tend (org-float-time tend)))
> -    (remove-text-properties (point-min) (point-max) '(:org-clock- 
> minutes t))
> +    (remove-text-properties (point-min) (point-max)
> +                            '(:org-clock-minutes t
> +                              :org-clock-force-headline-inclusion t))
>     (save-excursion
>       (goto-char (point-max))
>       (while (re-search-backward re nil t)
> @@ -1330,15 +1335,34 @@ TSTART and TEND can mark a time range to be  
> considered."
> 	  (setq t1 (+ t1 (string-to-number (match-string 5))
> 		      (* 60 (string-to-number (match-string 4))))))
> 	 (t ;; A headline
> -	  (setq level (- (match-end 1) (match-beginning 1)))
> -	  (when (or (> t1 0) (> (aref ltimes level) 0))
> -	    (loop for l from 0 to level do
> -		  (aset ltimes l (+ (aref ltimes l) t1)))
> -	    (setq t1 0 time (aref ltimes level))
> -	    (loop for l from level to (1- lmax) do
> -		  (aset ltimes l 0))
> -	    (goto-char (match-beginning 0))
> -	    (put-text-property (point) (point-at-eol) :org-clock-minutes  
> time)))))
> +	  (let* ((headline-forced
> +                  (get-text-property (point)
> +                                     :org-clock-force-headline- 
> inclusion))
> +                 (headline-included
> +                  (or (null headline-filter)
> +                      (save-excursion
> +                        (save-match-data (funcall headline- 
> filter))))))
> +	    (setq level (- (match-end 1) (match-beginning 1)))
> +	    (when (or (> t1 0) (> (aref ltimes level) 0))
> +	      (when (or headline-included headline-forced)
> +                (if headline-included
> +                    (loop for l from 0 to level do
> +                          (aset ltimes l (+ (aref ltimes l) t1))))
> +		(setq time (aref ltimes level))
> +		(goto-char (match-beginning 0))
> +		(put-text-property (point) (point-at-eol) :org-clock-minutes time)
> +                (if headline-filter
> +                    (save-excursion
> +                      (save-match-data
> +                        (while
> +                            (> (funcall outline-level) 1)
> +                          (outline-up-heading 1 t)
> +                          (put-text-property
> +                           (point) (point-at-eol)
> +                           :org-clock-force-headline-inclusion  
> t))))))
> +	      (setq t1 0)
> +	      (loop for l from level to (1- lmax) do
> +		    (aset ltimes l 0)))))))
>       (setq org-clock-file-total-minutes (aref ltimes 0)))
>     (set-buffer-modified-p bmp)))
>
> @@ -1658,6 +1682,8 @@ the currently selected interval size."
> 	   (te (plist-get params :tend))
> 	   (block (plist-get params :block))
> 	   (link (plist-get params :link))
> +	   (tags (plist-get params :tags))
> +	   (matcher (if tags (cdr (org-make-tags-matcher tags))))
> 	   ipos time p level hlc hdl tsp props content recalc formula pcol
> 	   cc beg end pos tbl tbl1 range-text rm-file-column scope-is-list  
> st)
>       (setq org-clock-file-total-minutes nil)
> @@ -1739,7 +1765,14 @@ the currently selected interval size."
> 	(goto-char pos)
>
> 	(unless scope-is-list
> -	  (org-clock-sum ts te)
> +	  (org-clock-sum ts te
> +			 (unless (null matcher)
> +			   (lambda ()
> +			     (let ((tags-list
> +				    (org-split-string
> +				     (or (org-entry-get (point) "ALLTAGS") "")
> +				     ":")))
> +			       (eval matcher)))))
> 	  (goto-char (point-min))
> 	  (setq st t)
> 	  (while (or (and (bobp) (prog1 st (setq st nil))
> -- 
> 1.6.6.1
>
> _______________________________________________
> 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