Re: NonGNU ELPA package submission: EPIO

[Stefan Monnier]

Daniel Hutzley [2022-04-01 11:59:05] wrote:
> I apologise for the vagueness of the README, I'll update it when I
> have the chance (I forgot to include the license footer anyway, so
> that'll be updated). While it is similar interface-wise to the
> PlatformIO-Mode they link on their page (again, I'll update the README
> with links when I get the chance), the internals are almost completely
> different (this plugin has no dependencies outside of EMACS, is
> documented in TexInfo, etc.). As for what PlatformIO is, it is a free
> (their writings do suggest an ideological rationale here) system for
> developing on embedded platforms such as Arduino (distinctly not
> web-based). Their CLI tool (Apache-2.0 license) is used, which does
> not depend on a server (after a bit of additional research, opt-out
> telemetry does exist, though it is documented, minimal, togglable, and
> auditable in the source code. I'll include this in the edit) outside
> of those commands which would not function without (namedly, the
> commands for the features listed on
> https://docs.platformio.org/en/latest/plus/pio-account.html and the
> system for managing libraries for interacting with different boards).
> I will update the documentation to cover all of this, along with a
> setting which disables telemetry using an environment variable and
> licensing info. Are there any other changes that are blocking here?

I see you distribute this code under the MPL-2.0.
I'm not very familiar with that license but a brief web search suggests
this is not compatible with the GPL, which makes it arguably "illegal"
to distribute your code (since it can't be used without linking it with
GPL code).

Any chance you could change it to GPLv3+?

Also see below the patch resulting from my "destructive" reading habits.

I think integration with `project` would be the most important feature.


        Stefan


diff --git a/epio.el b/epio.el
index 60b56400cb..dbf7156819 100644
--- a/epio.el
+++ b/epio.el
@@ -1,4 +1,4 @@
-;;; epio.el --- PlatformIO support for EMACS
+;;; epio.el --- PlatformIO support for EMACS  -*- lexical-binding: t; -*-
 
 ;; Copyright (C) 2022 Danielle Hutzley
 
@@ -16,9 +16,11 @@
 
 ;;; Commentary:
 
-;; This is a minor mode designed to make working with PlatformIO projects 
easier.
-;; It is loosely based off of ZachMassia/PlatformIO-Mode, but uses standard 
EMACS
-;; functionality to avoid dependencies.
+;; This is a minor mode designed to make working with PlatformIO projects
+;; easier.  It is loosely based off of ZachMassia/PlatformIO-Mode, but uses
+;; standard Emacs functionality to avoid dependencies.
+
+;; TODO: Integration with `project'?
 
 ;;; Code:
 (require 'compile)
@@ -31,12 +33,10 @@
 
 (defcustom epio-mode-prefix (kbd "C-c i")
   "EPIO keymap prefix."
-  :group 'epio
   :type  'key-sequence)
 
 (defcustom epio-pio-path "pio"
   "The path of the PlatformIO command-line tool."
-  :group 'epio
   :type  'string)
 
 ;;; Compilation mode
@@ -51,7 +51,7 @@
 
 (define-compilation-mode epio-compilation-mode "EPIO"
   "EPIO compilation mode."
-  (add-hook 'compilation-filter-hook 'epio-compilation-filter nil t))
+  (add-hook 'compilation-filter-hook #'epio-compilation-filter nil t))
 
 ;;; Auto-enable
 
@@ -65,12 +65,15 @@
 (defun epio--platformio-command (project &rest arguments)
   "Run PlatformIO in the given PROJECT with the given ARGUMENTS."
   (let* ((default-directory (epio--get-pio-project project))
+         ;; FIXME: seq-reduce + concat is O(N²), better use `mapconcat'.
+         ;; FIXME: What about spaces or other special chars in `arguments'?
+         ;; You should likely pass those through `shell-quote-argument'.
          (arguments-with-spaces (seq-reduce (lambda (acc elt) (concat acc " " 
elt)) arguments ""))
          (command (concat "pio -c EPIO" arguments-with-spaces)))
     (when (null default-directory)
       (user-error "Not in PlatformIO project!"))
     (save-some-buffers (current-buffer))
-    (compilation-start command 'epio-compilation-mode)))
+    (compilation-start command #'epio-compilation-mode)))
 
 (defmacro epio--defcommand (name docstring arguments)
   "Helper to create an EPIO command NAME using the given ARGUMENTS and 
DOCSTRING."
@@ -106,11 +109,10 @@
               ("i" epio-init+update-workspace)))
     map)
   "EPIO keymap after `epio-mode-prefix'.")
-(fset 'epio-command-map epio-command-map)
 
 (defvar epio-mode-map
   (let ((map (make-sparse-keymap)))
-    (define-key map epio-mode-prefix 'epio-command-map)
+    (define-key map epio-mode-prefix epio-command-map)
     map)
   "Keymap for EPIO mode.")
 
@@ -120,10 +122,8 @@
 ;;;###autoload
 (define-minor-mode epio-mode
   "PlatformIO support for EMACS."
-  :lighter " EPIO"
-  :keymap epio-mode-map
   :group 'epio
-  :require 'epio)
+  :lighter " EPIO")
 
 (provide 'epio)
 ;;; epio.el ends here