nikclayton / ob-sql-mode

sql-mode backend for Org Babel
GNU General Public License v3.0
32 stars 10 forks source link

Use sql-connection-alist for connections #3

Closed conornash closed 6 years ago

conornash commented 7 years ago

Is it possible to select an entry from the Emacs variable sql-connection-alist for connection details? This would make this package super useful for me, as all my SQL connection data is stored in a private file. That way I could share the queries and their results without sharing the credentials.

indigoviolet commented 6 years ago

This would be awesome.

stardiviner commented 6 years ago

I think it is possible. manage the variable sql-connection-alist and ob-sql-mode already has initialize product session function in https://github.com/nikclayton/ob-sql-mode/blob/master/ob-sql-mode.el#L207 ~org-babel-sql-mode-initiate-session, in Org-mode keybinding it is [C-c C-v C-z]. Need to push new connection to sql-connection-alist.

stardiviner commented 6 years ago

After test, seems org-babel-sql-mode-initiate-session has error:

Debugger entered--Lisp error: (wrong-number-of-arguments (1 . 1) 2)
  org-babel-sql-mode-initiate-session("test" ((:results . "replace") (:exports . "code") (:session . "test") (:product . "postgres") (:comments . "links") (:padline . "true") (:mkdirp . "yes") (:tangle . "no") (:noweb . "no") (:hlines . "no") (:cache . "yes")))
  funcall(org-babel-sql-mode-initiate-session "test" ((:results . "replace") (:exports . "code") (:session . "test") (:product . "postgres") (:comments . "links") (:padline . "true") (:mkdirp . "yes") (:tangle . "no") (:noweb . "no") (:hlines . "no") (:cache . "yes")))
stardiviner commented 6 years ago

Seems not compatible org-mode's org-babel-initiate-session command. argument number is wrong. I will dive into the source code to check out.

stardiviner commented 6 years ago

I fixed it, created PR at here https://github.com/nikclayton/ob-sql-mode/pull/4

nikclayton commented 6 years ago

Thanks, I've merged that in and pushed. If my understanding's correct this should show up on MELPA pretty soon.