packages submissions : org-sql-session and org-blog

packages submissions : org-sql-session and org-blog

[Phil Estival]

Hello,

Here are two packages that I'd like to submit to ELPA, with
the advise and approval of emacs-devel.

- 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.

- 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.

Questions and Criticism are welcome,
along enthusiasm and motivation.

Cheers,

Phil

Re: packages submissions : org-sql-session and org-blog

[Adam Porter]

Hi Phil,

A quick, initial suggestion: it would be good to reformat the Lisp code 
in both projects, as I see several cases of incorrect indentation, 
hanging parens, extra blank lines, etc, and some of them would make the 
code hard to read.  For my Lisp editing, I use aggressive-indent-mode, 
which ensures that the code is always indented correctly.  For some 
other reformatting, Oleh Krehel's Lispy package has commands to help.

Also, for org-sql-session (which sounds very useful), it would be good 
to compare and contrast it with the package it's meant to supersede. 
And since it's meant to integrate with org-babel, it might be good to 
discuss its inclusion on the Org mailing list also.

--Adam

Re: packages submissions : org-sql-session and org-blog

[Philip Kaludercic]

[-- Attachment #1: Type: text/plain, Size: 427 bytes --]

Phil Estival <pe@7d.nz> 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:


[-- Attachment #2: Type: text/plain, Size: 8544 bytes --]

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 @@
 
 ;; Copyright (C) 2024 Free Software Foundation, Inc.
 
-;; Author: Phil Estival pe@7d.nz
-;; Package-Requires: ((emacs "27.2")) ((org "9.5"))
+;; Author: Phil Estival <pe@7d.nz>
+;; Maintainer: Phil Estival <pe@7d.nz>
+;; Package-Requires: ((emacs "27.2") (org "9.5"))
 ;; Keywords: literate programming, reproducible research
 ;; URL: http://github.com/flintforge/ob-sql-session
 
-;; This file is NOT part of GNU Emacs.
+;; This file is part of GNU Emacs.
 
 ;; 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.
-;;
 
 ;; 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 as described in (elisp) Composite Types is useful!
+  :group 'org-babel			;perhaps create a new sub-group?
   :safe t)
 
 (defconst org-babel-header-args:sql-session
@@ -109,12 +109,10 @@
                             "-P" "footer=off"
                             "-A"
                             ))
-:
 
 (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)
 
     ;; Substitute $vars in body with the associated value. (See also s-format).
-    (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)))
 
     (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 engine))
       ;; 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: "‘replace-regexp’ is for interactive use only; use ‘re-search-forward’ and ‘replace-match’ instead."!
       (replace-regexp ;; clear the output or prompt and termination
        (sql-get-product-feature engine :ob-sql-session-clear-output) "")
 
       ;; 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)))))))
 
 
-(defun org-babel-sql-session-connect (engine params session)
+(defun org-babel-sql-session-connect (engine params session) ;please document 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 server
-         (buffer-name (format "%s"(if (string= session "none") ""
-																		(format "[%s]" session))))
+         (buffer-name (format "%s" (if (string= session "none") ""
+				     (format "[%s]" session))))
          ;; (buffer-name (format "%s%s://%s%s/%s"
          ;;                      (if (string= 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))
 
         ;; set the redirection filter
@@ -284,9 +279,9 @@ should also be prompted."
 
       ;; 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 "\\)"))))
 
       ;; 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."
 
           ;; 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-mode")
 ;; or (customize-variable 'org-structure-template-alist)
 
+;; LocalWords: sql org-mode session
 
 (provide 'ob-sql-session)
-
-;; LocalWords: sql org-mode session
 ;;; ob-sql-session.el ends here

[-- Attachment #3: Type: text/plain, Size: 2875 bytes --]


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

Re: packages submissions : ob-sql-session and org-blog

[Phil Estival]

* [2024-09-11 05:01] Adam Porter:
> Hi Phil,
> 
> A quick, initial suggestion: it would be good to reformat the Lisp code 
> in both projects, as I see several cases of incorrect indentation, 
> hanging parens, extra blank lines, etc, and some of them would make the 
> code hard to read.  For my Lisp editing, I use aggressive-indent-mode, 
> which ensures that the code is always indented correctly.  For some 
> other reformatting, Oleh Krehel's Lispy package has commands to help.

Hi Adam,

> Also, for org-sql-session (which sounds very useful), it would be good
> to compare and contrast it with the package it's meant to supersede.

The following comparison was added to the readme file.

ob-sql-mode :
- is very simple : forward the sql source through `sql-redirect'
- has a test suite
- but gives clunky output
- no :results table
- sql client shell commands messes up output
- prompt again for connection parameters when restarting a session

ob-sql-session :
- handle large results
- has :results tables
- accept header variables (:var)
- accept sql client shell commands
- keep login parameters
- prompt only for blank connection parameters
- can use `with-environment-variables'
- provide some more tests

> since it's meant to integrate with org-babel, it might be good to 
> discuss its inclusion on the Org mailing list also.

CC-ing the Org mailing list.

Phil

Re: packages submissions : ob-sql-session and org-blog

[Phil Estival]

* [2024-09-11 09:24] Philip Kaludercic:
> Phil Estival <pe@7d.nz> writes:
>> Hello
on ob-sql-session
>> 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?

I think so. The ownership transferred to the FSF and a copyright
assignment. One word about the license notice: I'm in agreement
with Stephan's opinion:
> historically we have accepted both "is part of GNU Emacs" and "is NOT
> part of GNU Emacs".  IMO the better choice is to say neither.

on org-blog :
> 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.
> 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,

> 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 :/

Understood. I'll be back with something cleaner, reconsider
where it can rely on the existing structures, and seek more advice
on the Org mailing list. Thanks for everything.

Phil

Re: packages submissions : ob-sql-session and org-blog

[Adam Porter]

Hi Phil,

On 9/17/24 05:00, Phil Estival wrote:

>> Also, for org-sql-session (which sounds very useful), it would be good
>> to compare and contrast it with the package it's meant to supersede.
> 
> The following comparison was added to the readme file.
> 
> ob-sql-mode :
> - is very simple : forward the sql source through `sql-redirect'
> - has a test suite
> - but gives clunky output
> - no :results table
> - sql client shell commands messes up output
> - prompt again for connection parameters when restarting a session
> 
> ob-sql-session :
> - handle large results
> - has :results tables
> - accept header variables (:var)
> - accept sql client shell commands
> - keep login parameters
> - prompt only for blank connection parameters
> - can use `with-environment-variables'
> - provide some more tests
> 
>> since it's meant to integrate with org-babel, it might be good to 
>> discuss its inclusion on the Org mailing list also.
To be clear, this makes 3 implementations of SQL support for Org Babel 
that I know of:

1. The built-in Org Babel SQL support, aka ob-sql, documented at 
<https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-sql.html>

2. ob-sql-mode, hosted at <https://github.com/nikclayton/ob-sql-mode>, 
which advertises itself as "an alternative backend for Org-Babel SQL SRC 
blocks that uses sql-mode to evaluate the query instead of Org-Babel's 
built-in SQL backends."

3. Your new ob-sql-session library.

You've mentioned how your library compares to ob-sql-mode, but how does 
it compare to the built-in ob-sql library?

Thanks,
Adam

Re: package submission : ob-sql-session

[Phil Estival]

Hi Adam,

* [2024-09-18 00:25] Adam Porter:
>
> Hi Phil,
> 
> On 9/17/24 05:00, Phil Estival wrote:
> 
>>> Also, for org-sql-session (which sounds very useful), it would be good
>>> to compare and contrast it with the package it's meant to supersede.
>>
>> The following comparison was added to the readme file.
>>
>> ob-sql-mode :
>> - is very simple : forward the sql source through `sql-redirect'
>> - has a test suite
>> - but gives clunky output
>> - no :results table
>> - sql client shell commands messes up output
>> - prompt again for connection parameters when restarting a session
>>
>> ob-sql-session :
>> - handle large results
>> - has :results tables
>> - accept header variables (:var)
>> - accept sql client shell commands
>> - keep login parameters
>> - prompt only for blank connection parameters
>> - can use `with-environment-variables'
>> - provide some more tests
>>
>>> since it's meant to integrate with org-babel, it might be good to 
>>> discuss its inclusion on the Org mailing list also.
> To be clear, this makes 3 implementations of SQL support for Org Babel 
> that I know of:
> 
> 1. The built-in Org Babel SQL support, aka ob-sql, documented at 
> <https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-sql.html>
> 
> 2. ob-sql-mode, hosted at <https://github.com/nikclayton/ob-sql-mode>, 
> which advertises itself as "an alternative backend for Org-Babel SQL SRC 
> blocks that uses sql-mode to evaluate the query instead of Org-Babel's 
> built-in SQL backends."
> 
> 3. Your new ob-sql-session library.
> 
> You've mentioned how your library compares to ob-sql-mode, but how does 
> it compare to the built-in ob-sql library?
> 

Here is a comparison with ob-sql-mode

ob-sql-session exists is for session support, which is in the TODOs
of ob-sql.

- ob-sql command execution relies on org-babel-eval
   (→ process-file → call-process).

- ob-sql-session runs an inferior process (in which
   sqli-interactive-mode can be activated when needed).
   The process output is filtered (e.g. results and prompts).
   When a session is demanded, this shell stays open for further commands
   and can keep a state (typically, when given special SQL commands).

|-----------+----------------------------+----------------------------|
|           | ob-sql                     | ob-sql-session             |
|-----------+----------------------------+----------------------------|
| Feat.     | - cmdline                  | - support for sessions     |
|           | - colnames as header arg   | - optionnal colnames       |
|-----------+----------------------------+----------------------------|
| TODO      |                            |                            |
|-----------+----------------------------+----------------------------|
|           | - support for sessions     | - colnames as header arg   |
|           | - support for more engines | - support for more engines |
|-----------+----------------------------+----------------------------|
| engines   |                            |                            |
| supported |                            |                            |
|-----------+----------------------------+----------------------------|
|           | - mysql                    | - Postgresql               |
|           | - dbi                      | - sqlite                   |
|           | - mssql                    |                            |
|           | - sqsh                     |                            |
|           | - postgresql               |                            |
|           | - oracle                   |                            |
|           | - vertica                  |                            |
|           | - saphana                  |                            |
|-----------+----------------------------+----------------------------|

ob-sql-session adapts sql-connect of sql.el by declaring
ob-sql-connect, in order to prompt only for missing connection
parameters.

- ob-sql defines org-babel-sql-dbstring-[engine]
    to be provided on a shell command line.

- likewise, ob-sql-session has to define
    - a connection string,
    - the prompt,
    - and the terminal command prefix
      for a every supported SQL client shell (or "engines")

- ob-sql-session requires sql.el.
   With the above defined, it should be compatible
   with most database of the sql.el's zoo. maybe.

Phil

Re: packages submissions : ob-sql-session and org-blog

[Philip Kaludercic]

Phil Estival <pe@7d.nz> writes:

> * [2024-09-11 09:24] Philip Kaludercic:
>> Phil Estival <pe@7d.nz> writes:
>>> Hello
> on ob-sql-session
>>> 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?
>
> I think so. The ownership transferred to the FSF and a copyright
> assignment. 

Right.

>             One word about the license notice: I'm in agreement
> with Stephan's opinion:
>> historically we have accepted both "is part of GNU Emacs" and "is NOT
>> part of GNU Emacs".  IMO the better choice is to say neither.

Yeah, that is fine.

> on org-blog :
>> 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.
>> 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,
>
>> 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 :/
>
> Understood. I'll be back with something cleaner, reconsider
> where it can rely on the existing structures, and seek more advice
> on the Org mailing list. Thanks for everything.

Thank you, and I hope my words were not too harsh.  I am not an
org-mode expert, perhaps the people on the org-mode mailing list might
have some comments and ideas on how to best implement your idea.

> Phil
>

-- 
	Philip Kaludercic on siskin

Re: packages submissions : ob-sql-session and org-blog

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > >> historically we have accepted both "is part of GNU Emacs" and "is NOT
  > >> part of GNU Emacs".  IMO the better choice is to say neither.

We can change this aspect of the license notice to fit the ways we
distribute that file.

If a file is part of Emacs, it should say so, and we should
write the notice as we do fo all the other files of Emacs.

If the file is NOT part of Emacs, meaning if it is distributed in
other ways separately from Emacs, them we could use the sort of
license notice that says "This program" and does not mention "GNU
Emacs".
-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)