Hi again!

It seems my mail got stuck in some filters - I sent one from another
address yesterday. Oh well, here it is anyway!

Comments below.

> From: Nicolas Goaziou [mailto:mail@nicolasgoaziou.fr]

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

Can't see what's wrong.. Autoindent by emacs did this. Anyways.. I
made the if-construct clearer by adding linebreaks.

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

Ok, fixed.

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

Fixed.

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

Fixed.

> > +      ; 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.

Trying to avoid it.

> > +  (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))))

Ok!

> > +(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

Fixed.

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

Fixed.

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

Fixed.

> > +      (while (not (memq char (append '(?\t ?\r ?/ ?. ?\ ?q)
> > +     (string-to-list tag-chars))))
>
> For clarity, use ?\s instead of ?\

Fixed.

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

Good point, changed.

> > +     ((eq char ?. )
>
> Spurious space.

Fixed.

> > +     ((or (eq char ?\ )
>
> See above.

Fixed.

>
> > +      (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.

Ok, fixed kind of. I don't want to compromise on the relatively long
variable names.

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

Fixed.

> >  @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
>

Indeed

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

Indeed again!

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

Thx!

> As a final step, would you mind adding tests
> for this?

Added a few now. Actually found an error in the way regexps were
matched and filtered due to the added tests. So, good point. The
errors are fixed ofc.

Fixed patches are attached,

Best regards
Gustav Wikström