pony-db-shell renames a random buffer to *PonyDbShell*

[gareth-rees]

Description

1. Visit mysite/myapp/models.py in one buffer.
2. Visit mysite/myapp/views.py in another buffer.
3. Run the command pony-db-shell.

What I expect to happen: a new buffer pops up containing a database shell.

What actually happens: the buffer containing models.py pops up, renamed to *PonyDbShell*

Analysis

This is caused by the interaction of three bugs:

1. In my settings.py I have 'ENGINE': 'django.db.backends.mysql' but Pony tests (equalp (pony-db-settings-engine db) "mysql")
2. If Pony can't find an appropriate sql-connect-* function to call, it calls (pony-pop "*SQL*") and (rename-buffer "*PonyDbShell*") anyway.
3. pony-pop calls (pop-to-buffer (get-buffer buffer)) which means that if the buffer does not exist, get-buffer returns nil, and pop-to-buffer applies its default behaviour: "If BUFFER-OR-NAME is nil, choose some other buffer."

   Fixes

I pushed a fix for this, but for the record, here's what I did:

Fixing stuff bottom-up, first pony-pop. (Note that this doesn't need an autoload cookie since it's not interactive: there is no reason for anyone to call it before pony-mode.el has been loaded.)

(defun* pony-pop (buffer-or-name &key dirlocals)
  "Select `buffer-or-name' in some window, as for `pop-to-buffer'.
Return the selected buffer if successful, or `nil' otherwise.

If the optional keyword argument `:dirlocals' is non-nil, set the
variable `dir-local-variables-alist' in the target buffer to be
equal to the value in the current buffer.  This is useful because
comint buffers without filenames associated will otherwise not
pick up directory-local settings."
  (let ((buffer (get-buffer buffer-or-name))
        (current-locals dir-local-variables-alist))
    (when buffer
      (pop-to-buffer buffer)
      (pony-mode)
      (and dirlocals
           current-locals
           (pony-local! 'dir-local-variables-alist current-locals)))
    buffer))

Second, pony-db-shell. I took the opportunity to make the following improvements:

1. Put the name of the created buffer in a variable to avoid repeating ourselves.
2. If the database shell buffer already exists, don't create a new one, just pop to the old one.
3. There's no need to change the global settings for sql-user and so on: we can use (let ...) to set them locally around the creation of the *SQL* buffer.
4. We don't need to maintain our own enumeration of SQL backends and which function to call: we can use sql-product-alist.
5. By calling (sql-product-interactive) we get features turned on via (sql-interactive-mode) including syntax highlighting,
6. Call rename-buffer with unique set to t so that the rename doesn't throw an error if the buffer already exists.
7. The use of the pony-db-settings structure is overkill since its only use is to get the data for pony-db-shell.

;;;###autoload
(defun pony-db-shell ()
  "Run interpreter for this project's default database as an inferior process."
  (interactive)
  (let ((buffer-name "*pony-db-shell*")
        (db-format (if (pony-setting-p "DATABASE_ENGINE") "DATABASE_%s"
                     "DATABASES['default']['%s']")))
    (if (comint-check-proc buffer-name)
        (pop-to-buffer buffer-name)
      (flet ((db-setting (name) (pony-get-setting (format db-format name)))
             ;; Rebind sql-get-login so that sql-product-interactive
             ;; doesn't try to prompt the user.
             (sql-get-login (&rest what)))
        (let* ((db-engine (db-setting "ENGINE"))
               (engine (car (last (split-string db-engine "\\."))))
               (sql-product
                (loop for product in sql-product-alist
                      if (search (symbol-name (car product)) engine)
                      return (car product)
                      finally do
                      (error "Don't know how to connect to %s" engine)))
               (sql-user (db-setting "USER"))
               (sql-password (db-setting "PASSWORD"))
               (sql-database (db-setting "NAME"))
               (sql-server (db-setting "HOST")))
          (sql-product-interactive)
          (when (pony-pop "*SQL*")
            (rename-buffer buffer-name t)))))))