davidmiller / pony-mode

Django mode for emacs
Other
149 stars 32 forks source link

pony-rc is over-complicated #36

Closed gareth-rees closed 12 years ago

gareth-rees commented 12 years ago

The function pony-rc reads as follows:

(defun pony-rc ()
  "Get The settings for the current project.

Read the current pony-project variable from the current buffer's .dir-locals.el"
  (let ((settings
         (if (memq 'pony-settings
                   (mapcar 'first dir-local-variables-alist))
             (cdr (find-if (lambda (x) (equal (first x) 'pony-settings))
                           dir-local-variables-alist))
           ;; For backwards compatibility we also allow ourselves to use .ponyrc
           (eval (pony-read-file (concat (pony-project-root) ".ponyrc"))))))
    (eval settings)))

Four questions:

  1. Why use equal when comparing against a symbol?
  2. Why not use the built-in function assq for looking up a key in an association list?
  3. In the .ponyrc case it looks the contents of the file get evaluated twice. Is this right?
  4. The docstring refers to the pony-project variable but what actually gets looked up is the pony-settings variable. Which is right?

If the double evaluation is wrong, the function could be rewritten like this:

(defun pony-rc ()
  "Get the settings for the current project.

Evaluate the pony-settings variable from the directory-local variables,
or if it does not exist, load .ponyrc instead."
  (eval (cdr (or (assq 'pony-settings dir-local-variables-alist)
                 (cons nil (pony-read-file (concat (pony-project-root) ".ponyrc")))))))