Mattias Engdegård <mattiase@acm.org> writes:

> +;; (rx (and line-start (regexp something-else))), statically or
> +;; (rx-to-string `(and line-start ,something-else)), dynamically.
>
> With your patch, the rx-to-string example should no longer be
> recommended, but eval is still of interest for compile-time
> substitution. What about:
>
> ;; (rx (and line-start (eval something-else))), statically or
> ;; (rx (and line-start (regexp something-else))), dynamically.

Not sure that we really want to get into the subtleties of static eval
in the intro examples.  I'm thinking we just drop the rx-to-string
example, without replacement.

> +         ;; Always group non string forms, since we can't be sure they
>
> "non-string forms"

Right.

> +(defun rx-literal (form)
> +  "Parse and produce code from FORM, which is `(literal STRING-EXP)'."
> +  (cond ((stringp form)
> +         ;; This is allowed(?), but makes little sense, you could just
> +         ;; use STRING directly.
>
> Yes, I did the same in ry. Maybe `literal' should be disallowed
> entirely in rx-to-string, since it's more likely to be a
> misunderstanding on the user's part.

I think disallowing it could potentially be annoying during development,
e.g., building incrementally in re-builder.

> +(defun rx-subforms (subforms &optional parent regexp-op)
>
> REGEXP-OP is perhaps better named SEPARATOR?

Yeah, especially since it's just the one "\\|" operator.

> +                  (cl-mapcan (lambda (x)
> +                               (cons regexp-op (funcall listify x)))
> +                             (cdr subregexps))))
> +          (t (cl-mapcan listify subregexps)))))
>
> Any reason for using cl-mapcan instead of straight mapcan?
> Not that it matters much.

I, um, didn't realize mapcan was builtin (when I saw mapcan elsewhere I
just assumed it was from cl.el).  In my defence, it's new since 26.1 :p

(In addition to the above two points, I've renamed this function to
rx--subforms, and re-arranged the code a bit)

> +`regexp' will not accept non-string arguments (so (literal
> +STRING) becomes just a more verbose version of STRING)."
>
> Try not breaking the line inside (literal STRING).

Right.