Multiple regexp replace in string

Question

I made a function that prompts me for values of variables (formatted %^{var1}) it found in a string and fills in said values.

(defun fill-var-in-string (STRING)
  "Find variables in a string, prompt for a value and fill in STRING."
  (let ((filled STRING))
    (while (string-match "%^{\\([^}]*\\)}" STRING (match-end 0))
      (setq filled (replace-regexp-in-string (format "%%^{%s}" (match-string 1 STRING))
                                             (read-string (format "%s: " (match-string 1 STRING)))
                                             filled)))
    filled))


(message "filled in: %s" (fill-var-in-string "(%^{size}, %^{center})")) 

It works, I'm wondering if I did it the lisp way. I did not make it interactive since for now I only will call this programmatically in org-mode files.

Answer 1

Style-wise, it looks odd to see the function argument named in all-caps. Standard convention is to use lower-case (but still upcase it in the doc-string).

---

Trying to run the code, it immediately failed:

Lisp error: (args-out-of-range "(%^{size}, %^{center})" 455)
  string-match("%^{\\([^}]*\\)}" "(%^{size}, %^{center})" 455)
  (while (string-match "%^{\\([^}]*\\)}" STRING (match-end 0)) (setq filled (replace-regexp-in-string (format "%%^{%s}" (match-string 1 STRING)) (read-string (format "%s: " (match-string 1 STRING))) filled)))
  (let ((filled STRING)) (while (string-match "%^{\\([^}]*\\)}" STRING (match-end 0)) (setq filled (replace-regexp-in-string (format "%%^{%s}" (match-string 1 STRING)) (read-string (format "%s: " (match-string 1 STRING))) filled))) filled)
  fill-var-in-string("(%^{size}, %^{center})")

This is clearly becase (match-end 0) could have any value at entry. I think we need another variable (start-pos):

(defun fill-var-in-string (string)
  "Find variables in a string, prompt for a value and fill in STRING."
  (let ((filled string)
        (start-pos 0))
    (while (string-match "%^{\\([^}]*\\)}" string start-pos)
      (setq filled (replace-regexp-in-string (format "%%^{%s}" (match-string 1 string))
                                             (read-string (format "%s: " (match-string 1 string)))
                                             filled)
            start-pos (match-end 0)))
    filled))

---

We can simplify a little, because (format "%%^{%s}" (match-string 1 string)) is exactly what we just found, so we can replace all that with (match-string 0 string):

(defun fill-var-in-string (string)
  "Find variables in a string, prompt for a value and fill in STRING."
  (let ((filled string)
        (start-pos 0))
    (while (string-match "%^{\\([^}]*\\)}" string start-pos)
      (setq filled (replace-regexp-in-string (match-string 0 string)
                                             (read-string (format "%s: " (match-string 1 string)))
                                             filled)
            start-pos (match-end 0)))
    filled))

---

I'm not convinced that we should be matching an empty name - I would use + rather than * in the regular expression there.

---

There's a subtle failure case. If the user provides a string that matches a replacement format, then it might get subsequently filled in (if it's a replacement we have yet to process). To avoid this, we shouldn't start replacing until we have read all the replacement strings - we'll need to populate an alist mapping variables to values, and then go through the string performing the replacements as a second phase.