Re: [O] [RFC] Org version of the Org manual

[Nicolas Goaziou]

Hello,

Achim Gratz <address@hidden> writes:

> Hi Tom,
>
> I have a patch that should fix your problems with some characters in
> macro expansions:
>
>
> From 27b22d17f629a50bd485a0320dac45616d7ceb7f Mon Sep 17 00:00:00 2001
> From: Achim Gratz <address@hidden>
> Date: Sun, 17 Mar 2013 10:20:10 +0100
> Subject: [PATCH] fix macro expansion with separators and backslashes
>
> * lisp/org-element.el (org-element-macro-parser): Do not try to
>   "repair bad splits", only split at the correct places.
> * lisp/org-macro.el (org-macro-expand): Do not try to interpret the
>   macro replacement text as a regex.
>
> Allow to write macros like {{{kbd(\\)}}} and {{{kbd(\\,)}}} and expand
> them correctly.  A backslash at the end of an argument was incorrectly
> trying to cons the next argument (which may not exist).
> ---
>  lisp/org-element.el | 16 +++++-----------
>  lisp/org-macro.el   |  2 +-
>  2 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/lisp/org-element.el b/lisp/org-element.el
> index ba2461a..337cad0 100644
> --- a/lisp/org-element.el
> +++ b/lisp/org-element.el
> @@ -3117,20 +3117,14 @@ (defun org-element-macro-parser ()
>         (post-blank (progn (goto-char (match-end 0))
>                            (skip-chars-forward " \t")))
>         (end (point))
> -       (args (let ((args (org-match-string-no-properties 3)) args2)
> +       (args (let ((args (org-match-string-no-properties 3)))
>                 (when args
>                   ;; Do not use `org-split-string' since empty
>                   ;; strings are meaningful here.
> -                 (setq args (split-string args ","))
> -                 (while args
> -                   (while (string-match "\\\\\\'" (car args))
> -                     ;; Repair bad splits, when comma is protected,
> -                        ;; and thus not a real separator.
> -                     (setcar (cdr args) (concat (substring (car args) 0 -1)
> -                                                "," (nth 1 args)))
> -                     (pop args))
> -                   (push (pop args) args2))
> -                 (mapcar 'org-trim (nreverse args2))))))
> +                 (setq args (replace-regexp-in-string "," "\000" args))
> +                 (setq args (replace-regexp-in-string "\\\\\000" "," args))
> +                 (setq args (split-string args "\000"))
> +                 (mapcar 'org-trim args)))))

I suggest the following code instead, which allows to escape the
escaping backslash so the comma is not escaped:

  (args (mapcar 'org-trim
                (split-string
                 (replace-regexp-in-string
                  "\\(\\\\+\\)?\\(,\\)"
                  (lambda (str)
                    (let ((slashes (match-string 1 str)))
                      (if (or (not slashes) (evenp (length slashes))) "\\1\000"
                        (concat (make-string (1- (length slashes)) ?\\) ","))))
                  (org-match-string-no-properties 3))
                 "\000")))

What do you think about it?

> @@ -137,7 +137,7 @@ (defun org-macro-expand (macro templates)
>                                 (org-element-property :args macro))
>                            ;; No argument: remove place-holder.
>                            ""))
> -                    template)))
> +                    template nil 'literal)))

I agree on that part.

Also, a test or two should be added to "test-org-element/macro-parser"
in test-org-element.el.


Regards,

-- 
Nicolas Goaziou