5
\$\begingroup\$

I've made a little function for myself that replaces certain variables to text in an org file. I declare a bunch of variables in the beginning of the file and then the function replaces all references to the variable with the value. So for example

#+VAR:location=AAAAAA

text text text \v{location} text text text

becomes

text text text AAAAAA text text text

It's my first time really playing with elisp and I'm wondering if I used the language correctly and if there's possibility to improvement.

  (defun mp/org-fill-variables ()
    "Fill in the variables"

    (interactive)

    (let ((variable-count 0))
      (goto-char 0)

      (while (search-forward-regexp "^#\\+VAR:" nil t)

        (set-mark-command nil)
        (when (search-forward "=")

          (backward-char)
          (let ((variable (buffer-substring (region-beginning) (region-end))))

            (forward-char)
            (set-mark-command nil)
            (end-of-line)
            (let ((value (buffer-substring (region-beginning) (region-end))))

              ;; todo check if value non-nil
              (replace-string-in-region (concat "\\v{" variable "}") value)
              (setq variable-count (1+ variable-count))))))

      (message "replaced %d variables" variable-count)))
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

You can make a better (interactive) declaration. Because this modifies the buffer, we want to disable it on read-only buffers. We do that this way:

    (interactive "*")

Consider making the the function work within the active region, rather than the whole buffer:

  (defun mp/org-fill-variables (start end)
    "Fill in the variables"
    (interactive "*r")

search-forward-regexp is an alias for built-in function re-search-forward. I would prefer the latter as that's more familiar to other programmers. At first, I thought you had used one of the interactive functions.

We shouldn't be meddling with user's mark or position, so wrap the whole function in (save-excursion ), and don't use the interactive functions (forward-char) or (end-of-line).

(goto-char 0) is almost always wrong. Use (goto-char (point-min)) so that we do the Right Thing when narrowing is in effect.

Instead of (buffer-substring), we can get the variable name from the match results. That also allows us to be more specific about what a variable name looks like (e.g. no newlines):

        (while (re-search-forward "^#\\+VAR:\\(.+\\)=\\(.+\\)" nil t)
          (let ((name (match-string 1))
                (value (match-string 2)))

Consider counting the number of replacements as well as the number of variables found.


Modified code:

  (defun mp/org-fill-variables (start end)
    "Fill in the variables."
    (interactive "*r")
    (save-excursion
      (let ((variable-count 0)
            (replacement-count 0))
        (goto-char start)
        (while (re-search-forward "^#\\+VAR:\\(.+\\)=\\(.+\\)" nil t)
          (let* ((name (match-string 1))
                 (value (match-string 2))
                 (replaced (replace-string-in-region (concat "\\v{" name "}") value
                                                     start end)))
            (setq variable-count (+ variable-count (if replaced 1 0))
                  replacement-count (+ replacement-count (or replaced 0)))))
        (message "Replaced %d variable(s) in %d location(s)"
                 variable-count replacement-count))))
\$\endgroup\$
5
  • \$\begingroup\$ Thanks, this is very helpful and clear. I made the function like I would do when I do it myself as it started with a macro, hence the use of 'forward-char', 'search-forward-regexp', ... \$\endgroup\$ Feb 17, 2023 at 12:04
  • \$\begingroup\$ I had some time to test this, but it does not work (copypasted). If no region is active it says The mark is not active now and when I select the whole buffer it says End after end of buffer. How do I start debugging this? \$\endgroup\$ Feb 17, 2023 at 15:46
  • 1
    \$\begingroup\$ Ah, that's the problem with (interactive "r") - it requires an active region. Perhaps make two functions, so that the buffer-wide one calls the region one with (point-min) (point-max) as arguments? \$\endgroup\$ Feb 17, 2023 at 16:25
  • 1
    \$\begingroup\$ And the "End after end of buffer" error is a bug on my part - because we modify the buffer, that invalidates end. We need to (copy-marker end) early on and use that marker instead. \$\endgroup\$ Feb 17, 2023 at 16:28
  • \$\begingroup\$ Just to be sure. I added (end (copy-marker end)) in the let variable declaration list \$\endgroup\$ Feb 20, 2023 at 8:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.