bug#36237: Support (rx (and (regexp EXPR) (regexp-quote EXPR)))

[Mattias Engdegård]

19 juni 2019 kl. 03.34 skrev Noam Postavsky <npostavs@gmail.com>:
> 
> Sure.  Here's the patch with regexp-quote change to literal, and
> rx--compile-to-lisp renamed.  I'll wait a bit more and push this weekend
> if there are no more comments.

Thank you. Some comments:

-;; (rx (and line-start (eval something-else))), statically or
-;; (rx-to-string '(and line-start ,something-else)), dynamically.
+;; (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.

(extra points if you change `and' into `seq')

+  (cond ((stringp form)
+         (rx-group-if (cadr form) rx-parent))
+        (rx--compile-to-lisp
+         ;; Always group non string forms, since we can't be sure they

"non-string forms"

+(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.
 
+(defun rx-subforms (subforms &optional parent regexp-op)

REGEXP-OP is perhaps better named SEPARATOR?

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

+NO-GROUP non-nil means don't put shy groups around the result.
+Note that unlike for the `rx' macro, subforms `literal' and
+`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).