From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id 6GJpGZPZQGDnYQAA0tVLHw (envelope-from ) for ; Thu, 04 Mar 2021 12:58:59 +0000 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id CGE7FZPZQGCjHgAA1q6Kng (envelope-from ) for ; Thu, 04 Mar 2021 12:58:59 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 191C221FA5 for ; Thu, 4 Mar 2021 13:58:58 +0100 (CET) Received: from localhost ([::1]:35288 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lHnZB-0005eQ-Qm for larch@yhetil.org; Thu, 04 Mar 2021 07:58:57 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:56728) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lHnW3-00032y-PE for emacs-orgmode@gnu.org; Thu, 04 Mar 2021 07:55:43 -0500 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:36683) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lHnW1-0006Su-IH for emacs-orgmode@gnu.org; Thu, 04 Mar 2021 07:55:43 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 5C4235C0161; Thu, 4 Mar 2021 07:55:39 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 04 Mar 2021 07:55:39 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=ixPvlo qOADAqmrrCsjTg3Vi5LvK308Zat6hLPO6dsrM=; b=bA0i0bgRqRwL+W8GMrmBnB dzxTrnQ2I6+/Zw4FpdwF30r/wCXCrrpbLh6dHKksv8G69/x7wiuwniKgmvNVx0c2 ee8liUiZh61BwJCZmwSTc7qRwEwxIIGC/T+lGwDk9km2kCzSJ12yVXB+OrNCFX1D tB0kEEMSvhyAZ5LLbUXNLtfWjUCU1TZmc+xPLcaDvdBJLQO0vM/saUl8Xs0/MOc5 hjO0WgWe1+j6c2+Qy18NmlxaIJGCSGOXpUdKQayytAvJmoanOEh9uANxwQ3Dh5iw yzBPomt6qDuurGgC7+iOMr6kC/A6q4y5lb7dJoDmzhI+qzreKsFt0awhSYvisBCw == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledruddtgedggeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepuffvfhfhkffffgggjggtsehmtderredtfeejnecuhfhrohhmpefpihgtkhcu ufgrvhgrghgvuceonhhitghksehnihgtkhhsrghvrghgvgdrtggrqeenucggtffrrghtth gvrhhnpeetjedukedvvddttdetudeghefggfeiteeggeejvefhleffhfefgedtvdevtefh veenucffohhmrghinhepghhnuhdrohhrghenucfkphepjedtrdehvddrvdegrddukedune cuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepnhhitghk sehnihgtkhhsrghvrghgvgdrtggr X-ME-Proxy: Received: from [192.168.0.67] (bras-base-aylmpq0104w-grc-16-70-52-24-181.dsl.bell.ca [70.52.24.181]) by mail.messagingengine.com (Postfix) with ESMTPA id EDB0524005E; Thu, 4 Mar 2021 07:55:38 -0500 (EST) Subject: Re: [PATCH] Reduce code duplication in ob-sql.el and ob-sqlite.el To: Kyle Meyer References: <0766206a-5df8-8d95-e6fd-2f96f06c9840@nicksavage.ca> <87eegvcwxz.fsf@kyleam.com> From: Nick Savage Message-ID: Date: Thu, 4 Mar 2021 07:55:38 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 MIME-Version: 1.0 In-Reply-To: <87eegvcwxz.fsf@kyleam.com> Content-Type: multipart/mixed; boundary="------------6E998784D91B37A65ADF65EE" Content-Language: en-US Received-SPF: none client-ip=66.111.4.28; envelope-from=nick@nicksavage.ca; helo=out4-smtp.messagingengine.com X-Spam_score_int: -25 X-Spam_score: -2.6 X-Spam_bar: -- X-Spam_report: (-2.6 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, NICE_REPLY_A=-0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emacs-orgmode@gnu.org Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: "Emacs-orgmode" X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -4.00 Authentication-Results: aspmx1.migadu.com; none X-Migadu-Queue-Id: 191C221FA5 X-Spam-Score: -4.00 X-Migadu-Scanner: scn0.migadu.com X-TUID: 1rHdO12VeAQb This is a multi-part message in MIME format. --------------6E998784D91B37A65ADF65EE Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Please see the attached updated patch with the changes requested. Nick On 3/4/21 12:25 AM, Kyle Meyer wrote: > Nick Savage writes: > >> Hi everyone, >> >> See the attached patch. It is a small change to reduce code duplication >> between ob-sql.el and ob-sqlite.el by reusing org-babel-sql-expand-vars >> as suggested by the FIXME in ob-sqlite.el. > Thank you. Looks good, though I think it'd be nice to keep > org-babel-sqlite-expand-vars around for a bit, marked as obsolete. > >> Subject: [PATCH] Reduce code duplication in ob-sqlite.el and ob-sql.el >> >> * lisp/ob-sqlite.el (org-babel-sqlite-expand-vars): removed function >> to replace with ob-sql.el version >> * lisp/ob-sql.el (org-babel-sql-expand-vars): updated to support >> expanding sqlite vars > Please capitalize the first word after ":" and end the entries with a > period. > > https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html > >> -(defun org-babel-sqlite-expand-vars (body vars) >> - "Expand the variables held in VARS in BODY." >> - ;; FIXME: Redundancy with org-babel-sql-expand-vars! >> - (mapc >> - (lambda (pair) >> - (setq body >> - (replace-regexp-in-string >> - (format "$%s" (car pair)) >> - (let ((val (cdr pair))) >> - (if (listp val) >> - (let ((data-file (org-babel-temp-file "sqlite-data-"))) >> - (with-temp-file data-file >> - (insert (orgtbl-to-csv val nil))) >> - data-file) >> - (if (stringp val) val (format "%S" val)))) >> - body))) >> - vars) >> - body) >> - > How about marking this with (declare (obsolete ...)) and keeping it > around as a wrapper that calls org-babel-sql-expand-vars? That will > give any third-party code that may have used this for whatever reason > (perhaps unlikely) a chance to update. --------------6E998784D91B37A65ADF65EE Content-Type: text/x-patch; charset=UTF-8; name="0001-Reduce-code-duplication-in-ob-sqlite.el-and-ob-sql.e.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="0001-Reduce-code-duplication-in-ob-sqlite.el-and-ob-sql.e.pa"; filename*1="tch" >From 1e2816c89a4fc87a8d01c20dfb5a3c4cf794b553 Mon Sep 17 00:00:00 2001 From: Nicholas Savage Date: Wed, 3 Mar 2021 07:47:15 -0500 Subject: [PATCH] Reduce code duplication in ob-sqlite.el and ob-sql.el * lisp/ob-sqlite.el (org-babel-sqlite-expand-vars): Marked function as obsolete. * lisp/ob-sql.el (org-babel-sql-expand-vars): Updated to support expanding sqlite vars. --- lisp/ob-sql.el | 15 +++++++++++---- lisp/ob-sqlite.el | 23 +++++------------------ 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/lisp/ob-sql.el b/lisp/ob-sql.el index 68d5ddd0a..b1c6920cb 100644 --- a/lisp/ob-sql.el +++ b/lisp/ob-sql.el @@ -350,8 +350,13 @@ SET COLSEP '|' (org-babel-pick-name (cdr (assq :rowname-names params)) (cdr (assq :rownames params)))))))) -(defun org-babel-sql-expand-vars (body vars) - "Expand the variables held in VARS in BODY." +(defun org-babel-sql-expand-vars (body vars &optional sqlite) + "Expand the variables held in VARS in BODY. + +If SQLITE has been provided, prevent passing a format to +`orgtbl-to-csv'. This prevents overriding the default format, which if +there were commas in the context of the table broke the table as an +argument mechanism." (mapc (lambda (pair) (setq body @@ -362,9 +367,11 @@ SET COLSEP '|' (let ((data-file (org-babel-temp-file "sql-data-"))) (with-temp-file data-file (insert (orgtbl-to-csv - val '(:fmt (lambda (el) (if (stringp el) + val (if sqlite + nil + '(:fmt (lambda (el) (if (stringp el) el - (format "%S" el))))))) + (format "%S" el)))))))) data-file) (if (stringp val) val (format "%S" val)))) body))) diff --git a/lisp/ob-sqlite.el b/lisp/ob-sqlite.el index 6e21fa9fd..332a29872 100644 --- a/lisp/ob-sqlite.el +++ b/lisp/ob-sqlite.el @@ -27,6 +27,7 @@ ;;; Code: (require 'ob) +(require 'ob-sql) (declare-function org-table-convert-region "org-table" (beg0 end0 &optional separator)) @@ -51,8 +52,8 @@ (defun org-babel-expand-body:sqlite (body params) "Expand BODY according to the values of PARAMS." - (org-babel-sqlite-expand-vars - body (org-babel--get-vars params))) + (org-babel-sql-expand-vars + body (org-babel--get-vars params) t)) (defvar org-babel-sqlite3-command "sqlite3") @@ -112,22 +113,8 @@ This function is called by `org-babel-execute-src-block'." (defun org-babel-sqlite-expand-vars (body vars) "Expand the variables held in VARS in BODY." - ;; FIXME: Redundancy with org-babel-sql-expand-vars! - (mapc - (lambda (pair) - (setq body - (replace-regexp-in-string - (format "$%s" (car pair)) - (let ((val (cdr pair))) - (if (listp val) - (let ((data-file (org-babel-temp-file "sqlite-data-"))) - (with-temp-file data-file - (insert (orgtbl-to-csv val nil))) - data-file) - (if (stringp val) val (format "%S" val)))) - body))) - vars) - body) + (declare (obsolete "use `org-babel-sql-expand-vars' instead." "Org 9.4")) + (org-babel-sql-expand-vars body vars t)) (defun org-babel-sqlite-table-or-scalar (result) "If RESULT looks like a trivial table, then unwrap it." -- 2.20.1 --------------6E998784D91B37A65ADF65EE--