Emacs column alignment command

Question

I'm making my first steps in ELisp programming and I want to know is there a better solution for this problem:

I have a file with two columns, that are separated by one or more spaces, ex.:

One 1
Two 2
Three 3
Four  4

I'd like to make some alignment (columns are separated by at least two spaces):

One    1
Two    2
Three  3
Four   4

My solution is:

(defun align-config-file ()
  "Align two-columned config file in a pretty manner"
  (interactive)
  (let* ((strings (split-string (buffer-string) "\n"))
         (data (mapcar (lambda (line) (split-string line " ")) strings))
         (first-col-lengths (mapcar (lambda (item) (length (car item))) data))
         (first-col-length (+ 2 (apply 'max first-col-lengths))))
    (erase-buffer)
    (while data
      (let* ((this-line (car data))
             (first-col (car this-line))
             (last-col (car (nthcdr (- (length this-line) 1) this-line)))
             (amount-of-spaces (- first-col-length (length first-col)))
             (spaces-inserted 0))
        (insert first-col)
        (while (< spaces-inserted amount-of-spaces)
          (insert " ")
          (set 'spaces-inserted (+ 1 spaces-inserted)))
        (insert last-col)
        (insert "\n"))
      (set 'data (cdr data)))))

It is not beautiful, but it works. Is it ok?

Answer 1

1. Design

The design is not very Emacs-like. In particular:

1. The command operates on the entire buffer contents. This means that if you want to line up some columns in a region (without affecting the rest of the buffer), you can't use your command. (At least not without copying the region to a new buffer, running the command, and then copying the result back again.)

   It would be better if the function operated on the contents of the current region: then it would be much more flexible.

2. Your command doesn't respect the current narrowing of the buffer. If you look at the docstring for erase-buffer, it says:

   Any narrowing restriction in effect (see narrow-to-region) is removed, so the buffer is truly empty after this.

   This would come as an unpleasant surprise to a user who had a narrowing in effect. It would be better to operate only on the narrowed part of the buffer, by calling

   (delete-region (point-min) (point-max))
   

   instead of (erase-buffer). (But in fact there are better ways to do it, which I'll describe below.)

3. Your command doesn't preserve the position of point. After running the command, point is always at the end of the buffer. It's always worth preserving the position of point (if appropriate) because that makes a function's behaviour predictable, and more useful in Lisp code and in macros.

2. Implementation

1. The command reads the entire buffer contents into strings, processes the strings in various ways, and then writes them back to the buffer.

   If you've recently come to Emacs Lisp from another language, then you're probably thinking that a buffer is a bit like a file, and so the right way to process it is to read the file contents into memory as string, process the contents, and then write it out.

   I can't stress enough that you won't get the full benefits of Emacs if you persist with this approach! A buffer in Emacs is not like a file. It's a highly flexible string-like data structure that supports a much wider set of operations than a string. So you should organize your code to operate directly on the buffer contents, using the same (or similar) functions that you would use as an interactive user.

2. There are some useful functions in the Emacs toolbox that you could use to simplify your code. If you look at the revised code you'll see that I made use of skip-chars-forward, the (poorly named) just-one-space, and (cl-loop ... maximize ...) to do maximization without constructing an intermediate list in memory.

3. Minor points

1. The name align-config-file seems wrong: it's not limited to configuration files, and it doesn't operate on a file (it operates on buffer contents). I think a better name would be something like align-two-columns.

2. The docstring says "in a pretty manner" which seems rather vague. What does it actually do?

3. The command ensures that there are at least two spaces between the columns. The number 2 seems rather arbitrary: why not make it an argument to the command, so that the user can supply it as a prefix argument?

4. Revised code

(Ask if you can't figure out how this works.)

(require 'cl-macs) ; for the "cl-loop" facility

(defun align-two-columns (begin end spaces)
  "Align two columns between BEGIN and END, ensuring that the
second column is vertically aligned, and there are SPACES spaces
between the columns."
  (interactive "r\np")
  (save-excursion
    (save-restriction
      (narrow-to-region begin end)
      (goto-char (point-min))
      ;; Determine the width of the first column.
      (let ((width (cl-loop while (not (eobp))
                            maximize (skip-chars-forward "^ \t\n")
                            do (forward-line 1))))
        ;; Back to the top and adjust the spaces.
        (goto-char (point-min))
        (while (not (eobp))
          (let ((col (skip-chars-forward "^ \t\n")))
            (unless (eolp)
              (just-one-space (+ spaces (- width col)))))
          (forward-line 1))))))

Answer 2

You should use dolist

(dolist (VAR LIST [RESULT]) BODY...)

and insert-char

(insert-char CHARACTER &optional COUNT INHERIT)

instead of the while constructs.

You can also insert more than one string with insert

(insert &rest ARGS)

You could use reduce instead of mapcar '(car + length + max).

(reduce FUNCTION SEQ [KEYWORD VALUE]...)

(reduce '(lambda (acc el)
           (max acc (length el)))
        '(("one" "two") ("three" "four") ("five" "six"))
        :initial-value 0
        :key 'car)
 → 5