Now that we have test coverage and a test runner, we can refactor the begin/end code.

First off, let’s namespace it. Instead of defun begin-end-quote (), let’s make it defun my/begin-end-quote ().

Why? There is another library with a function called begin-end-quote: I’m not using it, but if I were, after namespacing I wouldn’t have to worry about anything breaking because of the name collision.

The coding conventions suggest namespacing packages for widespread use with -, not / (q.v. typopunct-mode ’s typopunct-insert-quotation-mark), but namespacing “less formal” code with / seems a useful distinction. Sacha Chua suggested a further convention for Emacs config files of my/ instead of (say) sean/, since .emacs.d code is so often copied and borrowed and it would make it look more consistent after mixing code from multiple sources.

Run the tests: they still pass. Hurrah.

Almost all of the code between begin-end-verse and begin-end-quote is the same. I made them separate to begin with because I thought they would be more different, but they aren’t. So we can make everything but the outermost function take an argument of “quote” or “verse” and remove the duplication. (This will also let me quickly add variants like #+begin_src emacs-lisp and #+begin_example, which I’ve already needed while writing this blog.)

Again, run the tests, they still pass. Commit with this change.

Further quick points: newline does insert a carriage return, but insert “\n” would do the same thing, and feels more intuitive. Likewise, while previous-line and next-line do what they say, the more usual way would be to run forward-line with a positive or negative integer argument.

There is also duplication between my/begin-end-selected-region and my/begin-end-no-selected-region: they both print the #+begin and #+end tags, and if there’s a selected region it is reformatted, and if there isn’t, after printing the tags it moves the cursor back up between the tags. We can collapse the two methods into a single method with two conditionals.

(defun my/begin-end (variant)
(interactive)
(let ((cited-string "\n"))
(when (use-region-p)
(setq cited-string
(my/remove-old-citation-formatting (buffer-substring-no-properties (region-beginning) (region-end))))
(delete-region (region-beginning) (region-end)))
(insert "#+begin_" variant "\n"
cited-string
"#+end_" variant "\n"))
(unless (use-region-p)
(forward-line -2)))

The simpler conditional is at lines 11-12: (use-region-p) returns true if there is a region selected, and if there isn’t, it moves the cursor up two lines.

The one in lines 3-10 is more complicated.

In lines 3-7, we’re using a let statement to set cited-string initially to \n, which is what it should be if there’s no selection, and then we’re checking to see if there is a selected region: if there is, we’re resetting cited-string to the value of the selected region with the old formatting code stripped out, and then we’re deleting the selected region.

At the beginning of line 8, then, we’ve deleted the selected region if there was one, and until the end of the body of the let at the end of line 10, cited-string contains either a return character (if there was no selection) or the reformatted contents of the selected region (if there was a selection). Because of that, we can insert it between the tags either way.

As part of this, we also modified the my/fix-old-formatting method from one that dealt with the selected region in place to one which took a string and returned a modified string. All the selected region manipulation now takes place in the my/begin-end function.

Re-run the tests: they still pass. Excellent. Commit.

At this point, the refactoring is done, but it is now trivially easy to add new tests and implementations and key-bindings for #+begin_example / #+end_example and #+begin_src emacs-lisp / #+end_src.

The test for #+begin_src emacs-lisp breaks because for _src, the beginning tag and the end tag are different. The simplest way to get this passing is to pass in two arguments to my/begin-end, which for all but src will be identical, like so:

(defun my/begin-end-example ()
(interactive)
(my/begin-end "example" "example"))

(defun my/begin-end-src-emacs-lisp ()
(interactive)
(my/begin-end "src emacs-lisp" "src"))

(defun my/begin-end (begin-tag end-tag)
…
(insert "#+begin_" begin-tag "\n"
cited-string
"#+end_" end-tag "\n"))

At this point, the code does everything we need, the tests pass, and we drove out the new functionality we needed with tests. We’re done. Final commit.