Morgan Smith <[email protected]> writes:

> Patches 1-6 are basically the same as previously submitted (with more concise
> quote matching detection).

Fixed, on main, and on bugfix.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=fb3ccace2
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=818ea9b43
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=bbd008217
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8e161bccc
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c6168b4fd
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=04c089020
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=0e638c7e4

> Patches 7-10 are new and exciting.  Now we complete all the things!
> properties, tags, and even todo-keywords when it's appropriate.

I tried +TODO="<TAB> and got nothing.

> All of the code changes I want to make are here but there are a couple more
> documentation things that should be done.
> - News entry
> - Rename `org-tags-completion-function'.  What are we thinking?
>   `org-tags-matcher-completion-table'?  
> `org-tags-matcher-completion-function'?

`org-tags-matcher-completion-function'.
I think I explained why not table. Or was it not clear enough?

> Subject: [PATCH 01/10] Testing: New test
>  `test-org/org-tags-completion-function'
> ...
> Subject: [PATCH 02/10] org-tags-completion-function: Return correct boundary
> ...
> Subject: [PATCH 04/10] org-tags-completion-function: Link to info page in
>  docstring
> ...
> Subject: [PATCH 05/10] org-agenda-filter-completion-function: Return correct
>  boundary
> ...
> Subject: [PATCH 06/10] org-agenda-filter-completion-function: Add info page to
>  docstring

I applied these.

> Subject: [PATCH 03/10] org-tags-completion-function: Abort on unbalanced
>  quotes

What about TODO="<tab>?

> Subject: [PATCH 07/10] org-make-tags-matcher: Rewrite parsing regex using rx
> ...
> -              "&?"
> +              (? "&")

If we rewrite things with rx, the goal should be readability.
If so, I suggest using human-readable rx forms like (optional ...) and
also add more comments with examples of the match.

> Subject: [PATCH 08/10] org-make-tags-matcher: Simplify parsing logic

Looks reasonable from a cursory look.

> -       (let ((end (if (string-match "[-+:&,|]" suffix)
> +       (let ((end (if (string-match "[-+:&,|/]" suffix)

This is slightly suspicious. What if there is a / inside quoted value?

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Reply via email to