From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH] ob-sql.el: Option to reference connections in `sql-connection-alist' Date: Sun, 07 Apr 2019 09:24:26 +0200 Message-ID: <87sguu5lcl.fsf@nicolasgoaziou.fr> References: Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([209.51.188.92]:55148) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hD2AJ-00029p-Ux for emacs-orgmode@gnu.org; Sun, 07 Apr 2019 03:24:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hD2AI-0004o0-Un for emacs-orgmode@gnu.org; Sun, 07 Apr 2019 03:24:31 -0400 Received: from relay9-d.mail.gandi.net ([217.70.183.199]:50583) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hD2AI-0004mo-NG for emacs-orgmode@gnu.org; Sun, 07 Apr 2019 03:24:30 -0400 In-Reply-To: (Stefano Rodighiero's message of "Sun, 24 Mar 2019 13:04:33 +0100") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Stefano Rodighiero Cc: emacs-orgmode@gnu.org Hello, Stefano Rodighiero writes: > [This is the first patch I ever submitted. Thank you. Some comments follow. > I hope it complies with > your standards: if it does not, I'll be happy to work on it until it's > fine. I am not sure it qualifies as a tiny change.] It does. > Subject: [PATCH] ob-sql.el: Option to reference connections in > `sql-connection-alist' > > * ob-sql.el: Provide a new param called :dbconnection, that can be > used to reference connections defined in `sql-connection-alist', a > custom variable defined in sql.el. You need to spell out the new function in the commit message: * ob-sql.el (org-babel-find-db-connection-param): new function. This patch provides a new parameter :dbconnection, ... > +(defun org-babel-find-db-connection-param (params name) > + "Return db connection param NAME. database, parameter > +Given a param NAME, if :dbconnection is defined in PARAMS then > +look for the param into the corresponding connection defined in > +`sql-connection-alist`, otherwise look into PARAMS. Look > +`sql-connection-alist` (part of SQL mode) for how to define > +database connections." > + (if (assq :dbconnection params) > + (let* ((dbconnection (cdr (assq :dbconnection params))) > + (name-mapping '((dbhost . sql-server) > + (dbport . sql-port) > + (dbuser . sql-user) > + (dbpassword . sql-password) > + (database . sql-database))) > + (mapped-name (cdr (assq name name-mapping)))) > + (cadr (assq mapped-name > + (cdr (assoc dbconnection > + sql-connection-alist))))) > + (cdr (assq name params)))) Isn't there a type mismatch here? > + > (defun org-babel-execute:sql (body params) > "Execute a block of Sql code with Babel. > This function is called by `org-babel-execute-src-block'." > (let* ((result-params (cdr (assq :result-params params))) > (cmdline (cdr (assq :cmdline params))) > - (dbhost (cdr (assq :dbhost params))) > - (dbport (cdr (assq :dbport params))) > - (dbuser (cdr (assq :dbuser params))) > - (dbpassword (cdr (assq :dbpassword params))) > - (database (cdr (assq :database params))) > + (dbhost (org-babel-find-db-connection-param params 'dbhost)) > + (dbport (org-babel-find-db-connection-param params 'dbport)) > + (dbuser (org-babel-find-db-connection-param params 'dbuser)) > + (dbpassword (org-babel-find-db-connection-param params 'dbpassword)) > + (database (org-babel-find-db-connection-param params 'database)) > (engine (cdr (assq :engine params))) The change from keywords to plain symbols is slightly confusing. Could `org-babel-find-db-connection-param' use keywords instead? This is the usual format for parameters in Babel. Regards, -- Nicolas Goaziou