Re: ELPA submission: drepl (REPL protocol)

[Philip Kaludercic <philipk@posteo.net>]

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

Augusto Stoffel <arstoffel@gmail.com> writes:

> Hi again,
>
> I would like to submit the following package to ELPA:
>
>   https://github.com/astoff/drepl
>
> It provides alternative shells for the Python and Lua languages.  More
> importantly, however, it defines a framework and a protocol to create
> new REPLs which hopefully helps doing some things which are tricky in
> Comint (especially completion and multi-line editing).
>
> This is kind of an experiment at this point, but I would like to make it
> public and see what people think.

The idea seems interesting, I'll have to try it out at some point
(though I don't really use Python or Lua much, so I hope you plan to add
more languages in the future).

Code-wise, I just have a few minor comments:


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

diff --git a/drepl.el b/drepl.el
index 1c6bd71..1a11b47 100644
--- a/drepl.el
+++ b/drepl.el
@@ -4,6 +4,8 @@
 
 ;; Author: Augusto Stoffel <arstoffel@gmail.com>
 ;; Keywords: languages, processes
+;; Version: 1.0.0
+;; Package-Requires: ((emacs "29.1"))
 
 ;; 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
@@ -49,7 +51,7 @@
 (defface drepl-prompt-incomplete '((t :inherit (comint-highlight-prompt default)))
   "Face for continuation prompts when input is incomplete but valid.")
 
-(defface drepl-prompt-invalid '((t :inherit (error default)))
+(defface drepl-prompt-invalid '((t :inherit (error default))) ;why both?
   "Face for continuation prompts when input is invalid.")
 
 (defcustom drepl-directory #'drepl--project-directory
@@ -68,6 +70,8 @@ buffers, this is the dREPL buffer or nil.")
 
 ;;; Basic definitions
 
+;; Are classes needed here or could you use `cl-defstruct' with
+;; `:include'?
 (defclass drepl-base ()
   ((buffer :initarg :buffer :reader drepl--buffer)
    (status :initform nil :accessor drepl--status)
@@ -96,6 +100,8 @@ The message is formed by calling `format' with FMTSTRING and ARGS."
 
 ;;; Communication protocol
 
+;; 1. Mention /what/ is not available.
+;; 2. why not fall back to json.el?
 (defalias 'drepl--json-decode
   (if (json-available-p)
       (lambda (s)
@@ -413,7 +419,7 @@ if needed."
 (cl-defgeneric drepl--command (repl)
   "The command to start the REPL interpreter as a list of strings."
   (ignore repl)
-  (error "This needs an implementation"))
+  (error "This needs an implementation")) ;Mention what "this" is
 
 (cl-defgeneric drepl--init (repl)
   "Initialization code for REPL.

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


-- 
Philip Kaludercic