From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: packages submissions : org-sql-session and org-blog Date: Wed, 11 Sep 2024 07:24:14 +0000 Message-ID: <87zfoe64bl.fsf@posteo.net> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="40799"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Phil Estival Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Sep 11 09:25:22 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1soHj7-000ASB-Lr for ged-emacs-devel@m.gmane-mx.org; Wed, 11 Sep 2024 09:25:21 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1soHiF-0004yf-NB; Wed, 11 Sep 2024 03:24:27 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1soHiC-0004yO-6W for emacs-devel@gnu.org; Wed, 11 Sep 2024 03:24:25 -0400 Original-Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1soHi9-0008UQ-0H for emacs-devel@gnu.org; Wed, 11 Sep 2024 03:24:23 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id BCBB7240105 for ; Wed, 11 Sep 2024 09:24:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1726039456; bh=vYqeOzK6k2vO3N9upEhaDCK99X5BMezPIoPE1COhfFk=; h=From:To:Cc:Subject:Autocrypt:OpenPGP:Date:Message-ID:MIME-Version: Content-Type:From; b=GCgSgqbrFw80AzgaKs5/U9909tT34PMyduL8LoDfos/DIwEiZHbSOFN79CNhkV88o v7FVupSvkJvG6Snr7a5lVl02D1HGR3coXBPkjG9PJ1QLrEBJaEqMmq8D4GZZGYocKf nWVzowxI4my83K+5JMXwlZAthWm0FICbWZwWtmhq563hb06UhALC/v5kqS6hjB1IYx EOIb8pvoNAvBnbDHb3Y3oTNa7uDHxi8eaZH+abtcM+X15tAufSm31qoFxN3RYH6wxh LrypRD/CFNOzsdXxJ2IIq442G/2CPCIEED8WoelrSlZiHEVAF272eZTsXWmy7z5xsx JMCrHW/01REzg== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4X3XBR3tr6z9rxB; Wed, 11 Sep 2024 09:24:15 +0200 (CEST) In-Reply-To: (Phil Estival's message of "Tue, 10 Sep 2024 16:50:05 +0200") Autocrypt: addr=philipk@posteo.net; keydata= mDMEZBBQQhYJKwYBBAHaRw8BAQdAHJuofBrfqFh12uQu0Yi7mrl525F28eTmwUDflFNmdui0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiWBBMWCAA+FiEEDg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwMFCQHhM4AFCwkI BwIGFQoJCAsCBBYCAwECHgECF4AACgkQ8xYDWXahwulikAEA77hloUiSrXgFkUVJhlKBpLCHUjA0 mWZ9j9w5d08+jVwBAK6c4iGP7j+/PhbkxaEKa4V3MzIl7zJkcNNjHCXmvFcEuDgEZBBQQhIKKwYB BAGXVQEFAQEHQI5NLiLRjZy3OfSt1dhCmFyn+fN/QKELUYQetiaoe+MMAwEIB4h+BBgWCAAmFiEE Dg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwwFCQHhM4AACgkQ8xYDWXahwukm+wEA8cml4JpK NeAu65rg+auKrPOP6TP/4YWRCTIvuYDm0joBALw98AMz7/qMHvSCeU/hw9PL6u6R2EScxtpKnWof z4oM OpenPGP: id=philipk@posteo.net; url="https://keys.openpgp.org/vks/v1/by-email/philipk@posteo.net"; preference=signencrypt Received-SPF: pass client-ip=185.67.36.66; envelope-from=philipk@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:323539 Archived-At: --=-=-= Content-Type: text/plain Phil Estival writes: > Hello, > > Here are two packages that I'd like to submit to ELPA, with > the advise and approval of emacs-devel. Just to be sure, you mean GNU ELPA (not NonGNU ELPA), right? > - ob-sql-session : https://github.com/flintforge/ob-sql-session > meant as a replacement for the orphan package ob-sql-mode. > It's in decent state. Version 1. Here are a inline few comments and suggestions: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable diff --git a/ob-sql-session.el b/ob-sql-session.el index 96c0949..2a67c1c 100644 --- a/ob-sql-session.el +++ b/ob-sql-session.el @@ -2,12 +2,13 @@ =20 ;; Copyright (C) 2024 Free Software Foundation, Inc. =20 -;; Author: Phil Estival pe@7d.nz -;; Package-Requires: ((emacs "27.2")) ((org "9.5")) +;; Author: Phil Estival +;; Maintainer: Phil Estival +;; Package-Requires: ((emacs "27.2") (org "9.5")) ;; Keywords: literate programming, reproducible research ;; URL: http://github.com/flintforge/ob-sql-session =20 -;; This file is NOT part of GNU Emacs. +;; This file is part of GNU Emacs. =20 ;; This program is free software; you can redistribute it and/or modify ;; it under the terms of the GNU General Public License as published by @@ -46,7 +47,6 @@ ;; $ by the associated value. This does not declare new variables ;; (with a \set command for instance) as they would be stateful and ;; span over blocks in sessions. -;; =20 ;; In session mode, the connexion parameter are required only when ;; login, and no longer required for further requests. @@ -65,8 +65,8 @@ (defcustom org-babel-default-header-args:sql-session '((:engine . "sqlite")) "Default header args." - :type '(alist :key-type symbol :value-type string) - :group 'org-babel + :type '(alist :key-type symbol :value-type string) ;adding an :options a= s described in (elisp) Composite Types is useful! + :group 'org-babel ;perhaps create a new sub-group? :safe t) =20 (defconst org-babel-header-args:sql-session @@ -109,12 +109,10 @@ "-P" "footer=3Doff" "-A" )) -: =20 (defun org-babel-execute:sql-session (body params) "Execute SQL statements in BODY with PARAMS." - (let* ( - (processed-params (org-babel-process-params params)) + (let* ((processed-params (org-babel-process-params params)) (session (cdr (assoc :session processed-params))) (engine (cdr (assoc :engine processed-params))) (engine (intern (or engine (user-error "Missing :engine")))) @@ -128,10 +126,9 @@ (setq sql-product engine) =20 ;; Substitute $vars in body with the associated value. (See also s-for= mat). - (mapc - (lambda(v) (setq body (string-replace - (concat "$"(symbol-name(car v)))(cdr v) body))) - vars) + (dolist (v vars) + (setq body (string-replace + (concat "$"(symbol-name(car v)))(cdr v) body))) =20 (with-current-buffer (get-buffer-create "*ob-sql-result*") (erase-buffer)) @@ -142,6 +139,7 @@ (process-send-string (current-buffer) (ob-sql-format-query body engi= ne)) ;; todo: check org-babel-comint-async-register (while (not ob-sql-session-command-terminated) + ;; could there be a race condition here as described in (elisp) Accepting= Output? (sleep-for 0.03)) ;; command finished, remove filter (set-process-filter (get-buffer-process sql--buffer) nil) @@ -155,13 +153,13 @@ ;; get results (with-current-buffer (get-buffer-create "*ob-sql-result*") (goto-char (point-min)) + ;; Byte compiler hints: "=E2=80=98replace-regexp=E2=80=99 is for int= eractive use only; use =E2=80=98re-search-forward=E2=80=99 and =E2=80=98rep= lace-match=E2=80=99 instead."! (replace-regexp ;; clear the output or prompt and termination (sql-get-product-feature engine :ob-sql-session-clear-output) "") =20 ;; some client can also directly format to tables (when (member "table" results) ;;(org-table-convert-region (point-min)(point-max) "|") - ;;; or (goto-char (point-max)) (delete-char -1) ;; last newline (beginning-of-line) (let ((end (point))) @@ -180,15 +178,14 @@ is valid even when `sql-interactive-mode' isn't set. = BUFFER can be a buffer object or a buffer name. The buffer must be a live buffer, have a running process attached to it, and, if PRODUCT or CONNECTION are specified, its `sql-product' or `sql-connection' must match." - (let ((buffer (get-buffer buffer))) (and buffer (buffer-live-p buffer) (let ((proc (get-buffer-process buffer))) - (and proc (memq (process-status proc) '(open run ))))))) + (and proc (memq (process-status proc) '(open run))))))) =20 =20 -(defun org-babel-sql-session-connect (engine params session) +(defun org-babel-sql-session-connect (engine params session) ;please docum= ent the arguments! use checkdoc! "Start the SQL client of ENGINE if it has not in a buffer. Clear the intermediate buffer from previous output, and set the process filter. @@ -200,15 +197,13 @@ that clearly identifies the connexion from Emacs, to *SQL [session]* in order to retrieved a session with its name alone, the other parameters in the header args beeing no longer needed while the session stays open." - - (let* ((sql-database (cdr (assoc :database params))) (sql-user (cdr (assoc :dbuser params))) (sql-password (cdr (assoc :dbpassword params))) (sql-server (cdr (assoc :dbhost params))) ;; (sql-port (cdr (assoc :port params))) ;; to concat to the serv= er - (buffer-name (format "%s"(if (string=3D session "none") "" - (format "[%s]" session)))) + (buffer-name (format "%s" (if (string=3D session "none") "" + (format "[%s]" session)))) ;; (buffer-name (format "%s%s://%s%s/%s" ;; (if (string=3D session "none") "" ;; (format "[%s] " session)) @@ -249,7 +244,7 @@ no longer needed while the session stays open." ;; (set-process-filter sql-term-proc ;; #'ob-sql-session-comint-connect-filter) ;;(while (not ob-sql-session-connected)) - (sleep-for 0.06) + (sleep-for 0.06) ;The more or less arbitrary value here makes me = uncompfortable (with-current-buffer (get-buffer ob-sql-buffer) (erase-buffer)) =20 ;; set the redirection filter @@ -284,9 +279,9 @@ should also be prompted." =20 ;; store the regexp used to clear output (prompt1|indicator|prompt2) (sql-set-product-feature engine :ob-sql-session-clear-output - (concat "\\(" prompt-regexp "\\)" - "\\|\\(" ob-sql-session--batch-end-indicator "\n\\)" - (when prompt-cont-regexp (concat "\\|\\(" prompt-cont-= regexp "\\)")))) + (concat "\\(" prompt-regexp "\\)" + "\\|\\(" ob-sql-session--batch-end-indicator "\n\\)" + (when prompt-cont-regexp (concat "\\|\\(" prompt-cont-regexp "\= \)")))) =20 ;; Get credentials. ;; either all fields are provided @@ -326,7 +321,7 @@ should also be prompted." (let ((process-environment (copy-sequence process-environment)) (variables (sql-get-product-feature engine :environment))) (mapc (lambda (elem) ; environment variables, evaluated here - (setenv (car elem) (eval(cadr elem)))) + (setenv (car elem) (eval (cadr elem)))) variables) (funcall (sql-get-product-feature engine :sqli-comint-func) engine @@ -355,8 +350,8 @@ should also be prompted." =20 ;; no prompt, connexion failed (and process is terminated) (goto-char (point-max)) - (when (not (re-search-backward prompt-regexp 0 t)) - (user-error "Connexion failed"))) + (unless (re-search-backward prompt-regexp 0 t) + (user-error "Connection failed"))) ;is this a _user_ error? ;;(run-hooks 'sql-login-hook) ; don't ) (sql-progress-reporter-done rpt) @@ -370,7 +365,6 @@ Carefully separate client commands from SQL commands Concatenate SQL commands as one line is one way to stop on error. Otherwise the entire batch will be emitted no matter what. Finnally add the termination command." - (setq sql-product engine) (concat (let ((commands (split-string str "\n")) @@ -411,8 +405,7 @@ its message buffer" ;;(add-to-list 'org-structure-template-alist '("sql" . "src sql-session-mo= de") ;; or (customize-variable 'org-structure-template-alist) =20 +;; LocalWords: sql org-mode session =20 (provide 'ob-sql-session) - -;; LocalWords: sql org-mode session ;;; ob-sql-session.el ends here --=-=-= Content-Type: text/plain I agree with Adam that the code is... etwas messy ATM. Taking a moment to reindent everything and perhaps add a .dir-locals.el file that enforces a consistent code style would be of long-term use. > - Org-weblog (that I may rename org-blog): > https://gitlab.com/7dnz/org-weblog. Version 0. > Written in literate programming, yet by a lisp beginner > in that time. > The documentation goes along with the program itself and > they get both published as a static site. > Demo here: https://7d.nz/Blogging-with-org.html > The code is quite long. This paragraph gives the overall idea: > https://7d.nz/Blogging-with-org.html#Introduction. > Has a lot of space for improvements, options and customization, > but only a few for a v1. Hmm, I don't know if we have any literate programs on ELPA yet. If you prefer that style of development, that is fair, but what I would suggest would be to add a Makefile and extract the elisp at "compile-time", or in the case of ELPA when the tarball is being built. Evaluating the expression in org-weblog.el gives me an error Debugger entered--Lisp error: (void-function org-weblog-auto-export-mode) so I don't think that is ready yet. Nevertheless, skimming through the .org file (which most certainly should /not/ be the README btw) the code could also use some cleaning up, at least if the intention is to publish it as a generic package. The functions don't seem to adhere to a specific conventional namespace, Occur on "defun" gives me: --8<---------------cut here---------------start------------->8--- 319: (defun org-up-heading (level) 327: (defun heading-components-at-level (level) 366: (defun split-datetime+title (str) 375: (defun timestamp-and-title (str) 387: (defun org-html-title-only (str) 462: (defun replace-chars-in-string (set-1 with-set-2 str) 476: (defun replace-chars (set-1 char-2 str) 514: (defun sluggify (str) 540: (defun sluggif1 (str) 588: (defun append-next-numeral (string-list item) 623: (defun exporting-article-p (info) (plist-get info :export-article)) 625: (defun org-blog-headline (headline contents info) 761: (defun src-code-block (src-block contents info) 792: (defun blog-html-statistics-cookie (statistics-cookie _contents _info) 833: (defun org-blog--build-meta-info (info) 921: (defun org-blog-inner-template (contents info) 939: (defun org-blog-template (contents info) 1110: (defun org-blog--build-pre/postamble (type info) --8<---------------cut here---------------end--------------->8--- I have my doubts as to how much sense it would make to try and convert this personal code into a proper package, instead of starting over again :/ > Questions and Criticism are welcome, > along enthusiasm and motivation. > > Cheers, > > Phil > > -- Philip Kaludercic on siskin --=-=-=--