Re: [RFC] [PATCH] Changes to Tag groups - allow nesting and regexps

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Gustav Wikström <gustav.erik@gmail.com> writes:

> I've fixed the issues raised (IMO), and new patches are attached. I've
> added a patch for documentation also.

Thank you.

> I'd say it's an unnecessary limitation if group tags have to be
> exclusive on a headline. The more general case should be allowed and I
> can see use-cases for it.
>
> If you, for example, want to create a taxonomy of your tags and a part
> of your taxonomy is this:
>
> #+TAGS: [ CS : DB OS Software Versioning Programming BI ]
>
> What reason is there for Org mode to limit me to only choosing one of
> the above? Lets say I find an article on the web and want to create a
> node in my org-mode repository about it. Maybe linking the article and
> adding a few thoughts. The fictive article may be called "the
> importance of good database-design in Business intelligence". It seems
> two tags of the above would fit just fine; DB & BI.

Fair enough.

> +	     (taggroups (if downcased (mapcar (lambda (tg) (mapcar #'downcase tg))
> +					      taggroups) taggroups))

Nitpick: indentation

(taggroups (if downcased
               (mapcar (lambda (tg) (mapcar #'downcase tg)) taggroups) 
             taggroups))

> +		    (delq nil (mapcar (lambda (x)
> +					(if (stringp x)
> +					    (and (equal "{" (substring x 0 1))
> +						 (equal "}" (substring x -1))
> +						 x)
> +					  x)) tags-in-group))

Same here. TAGS-IN-GROUP should be at the same level as (lambda (x) ...)

> +		    regexp-in-group
> +		    (mapcar (lambda (x)
> +			      (substring x 1 -1)) regexp-in-group-escaped)

Ditto.

> +		    tags-in-group
> +		    (delq nil (mapcar (lambda (x)
> +					(if (stringp x)
> +					    (and (not (equal "{" (substring x 0 1)))
> +						 (not (equal "}" (substring x -1)))
> +						 x)
> +					                       x)) tags-in-group)))

Ditto.

> +	      ; If single-as-list, do no more in the while-loop...
> +	      (if (not single-as-list)
> +		  (progn
> +		    (if regexp-in-group
> +			(setq regexp-in-group (concat "\\|" (mapconcat 'identity regexp-in-group "\\|"))))
> +		    (setq tags-in-group (concat dir "{\\<" (regexp-opt tags-in-group) regexp-in-group  "\\>}"))

You need to keep lines within 80 columns.

> +			  (when (member tg g)
> +			    (mapc (lambda (x)
> +				    (setq current (delete x current)))
> +				  g)))

While you're at it:

  (when (member tg g) (dolist (x g) (setq current (delete x current))))

> +(defun org-agenda-filter-by-tag (arg &optional char exclude)
>    "Keep only those lines in the agenda buffer that have a specific tag.
>  The tag is selected with its fast selection letter, as configured.
> -With prefix argument STRIP, remove all lines that do have the tag.
> -A lisp caller can specify CHAR.  NARROW means that the new tag should be
> -used to narrow the search - the interactive user can also press `-' or `+'
> -to switch to narrowing."
> +With a single `C-u' prefix ARG, exclude the agenda search.  With a
> +double `C-u' prefix ARG, filter the literal tag. I.e. don't filter on
                                                  ^^^
                                             missing space

Also, instead of hard-coding `C-u', you could use \\[universal-argument]
within the doc string. See, for example, `org-tree-to-indirect-buffer'.

> +	 (exclude (if exclude exclude (equal arg '(4))))

  (exclude (or exclude (equal arg '(4))))

> +      (while (not (memq char (append '(?\t ?\r ?/ ?. ?\ ?q)
> +				     (string-to-list tag-chars))))

For clarity, use ?\s instead of ?\

Also, I suggest to move the consing before the while loop.

> +     ((eq char ?. )

Spurious space.

> +     ((or (eq char ?\ )

See above.

> +	      (save-match-data
> +		    (let (tags-expanded)
> +		      (dolist (x (cdr tags-in-group))
> +			(if (and (member x taggroups-keys)
> +				 (not (member x work-already-expanded)))
> +			    (setq tags-expanded (delete-dups
> +						 (append (org-tags-expand x t downcased work-already-expanded)
> +							 tags-expanded)))
> +			  (setq tags-expanded (append (list x) tags-expanded)))
> +			(setq work-already-expanded (delete-dups (append tags-expanded work-already-expanded))))
> +		      (setq tags-in-group (delete-dups (cons (car tags-in-group)
> +
> tags-expanded)))))

Lines too wide.

> +Tags can be defined in hierarchies. A tag can be defined as a @emph{group
                                     ^^^
                                  missing space

> +#+TAGS: [ Persp : Vision Goal AOF Project ]
> +#+TAGS: [ Control : Context Task ]
>  @end example
>  
> -In this example, @samp{@@read} is a @emph{group tag} for a set of three
> -tags: @samp{@@read}, @samp{@@read_book} and @samp{@@read_ebook}.
> +That can conceptually be seen as a hierarchy of tags:
>  
> -You can also use the @code{:grouptags} keyword directly when setting
> -@code{org-tag-alist}:
> +@example
> +- GTD
> +  - Persp
> +    - Vision
> +    - Goal
> +    - AOF
> +    - Project
> +  - Control
> +    - Context
> +    - Task
> +@end example
> +
> +You can use the @code{:startgrouptag}, @code{:grouptags} and
> +@code{:endgrouptag} keyword directly when setting @code{org-tag-alist}
> +directly:
>  
>  @lisp
> -(setq org-tag-alist '((:startgroup . nil)
> +(setq org-tag-alist '((:startgrouptag . nil)
>                        ("@@read" . nil)
>                        (:grouptags . nil)
>                        ("@@read_book" . nil)
>                        ("@@read_ebook" . nil)
> -                      (:endgroup . nil)))
> +                      (:endgrouptag . nil)))
>  @end lisp

The following is clearer

  @lisp
  (setq org-tag-alist '((:startgrouptag)
                        ("@@read")
                        (:grouptags)
                        ("@@read_book")
                        ("@@read_ebook")
                        (:endgrouptag)))
  @end lisp

However, shouldn't this example apply to the one above? (e.g.., with
Control : Context Task tag line)?

> +Searching for the tag Project will now list all tags also including regular
> +expression matches for P@@.+.  Similar for tag-searches on Vision, Goal and
> +AOF.  This can be good for example if tags for a certain project is tagged
> +with a common project-identifier, i.e. P@@2014_OrgTags.

@samp{Project} @samp{Vision}... @samp{P@@2014_OrgTags}.

This all looks very nice. As a final step, would you mind adding tests
for this?

As a starter `test-org/set-regexps-and-options' in "test-org.el" could
be extended to test parsing of square brackets tags.

Since you're touching some hairy part of Org, the more tests the better.


Regards,