Re: [NonGNU ELPA] New package: flymake-guile

[Philip Kaludercic <philipk@posteo.net>]

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

Distopico <distopico@riseup.net> writes:

> On 2023-09-01, Philip Kaludercic <philipk@posteo.net> wrote:
>
>> Distopico <distopico@riseup.net> writes:
>>
>>> On 2023-08-31, Philip Kaludercic <philipk@posteo.net> wrote:
>>>
>>>> Distopico <distopico@riseup.net> writes:
>>>>
>>>>> On 2023-08-31, Philip Kaludercic <philipk@posteo.net> wrote:
>>>>>
>>>>>> Distopico <distopico@riseup.net> writes:
>>>>>>
>>>>>>> Hi all!
>>>>>>>
>>>>>>> I'm the author of a new package `flymake-guile` and I
>>>>>>> would like to include it in Nongnu ELPA.
>>>>>>
>>>>>> Just to be sure, you are sure you don't want to include your package in
>>>>>> GNU ELPA?
>>>>>>
>>>>>>> Here the repo: https://framagit.org/flymake-backends/flymake-guile
>>>>>>
>>>>>> I am not familiar with the "flymake-quickdef" package, but it doesn't
>>>>>> seem to be much shorter than just defining a regular flymake backend.
>>>>>> As there have been some discussions wrt providing a kind of DSL for
>>>>>> Flymake backends, I am not sure if adding flymake-quickdef would be that
>>>>>> constructive at this point.  Would you consider updating your package to
>>>>>> not use the dependency?  You can check out other flymake-... modes in
>>>>>> GNU and NonGNU ELPA for inspiration.
>>>>>>
>>>>> Thank you for your feedback, For now I'm fine sending it to NonGNU ELPA,
>>>>> and for now I would like to keep `flymake-quickdef`, I have plans to
>>>>> write other backend and I don't wanna repeat the same validations and
>>>>> code over and over, I'll switch to the DLS when it is implemented.
>>>>
>>>> FWIW it already exists in this form https://github.com/mohkale/flymake-collection.
>>>>
>>>> And just to make sure, you are certain you want to implement this on top
>>>> of a DSL?  I have to admit that I am really not a fan of the way that
>>>> flymake-quickdef is implemented, but one redeeming feature appears to be
>>>> that you could macroexpand it away, then clean the code up.
>>>>
>>>
>>> flymake-collection use an adaptation of flymake-quickdef[1], that is
>>> a macro similar to the quickdef one[2], could be use quickdef a blocker
>>> to add the package on NonGNU ELPA?
>>
>> I am afraid I don't understand your question.
>
> My Question is if remove flymake-quickdef as dependency is a requirement
> to merge flymake-guile package into NonGNU ELPA or it's just a
> recommendation?   

It is a strong recommendation.  FWIW I have done the work of
macroexpanding and cleaning the resulting code up (but there is still
some more work to be done), and you can see what I had in mind:


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

diff --git a/flymake-guile.el b/flymake-guile.el
index d4b17b6..da64903 100644
--- a/flymake-guile.el
+++ b/flymake-guile.el
@@ -3,7 +3,7 @@
 ;; Copyright (c) 2023 Camilo Q.S. (Distopico) <distopico@riseup.net>
 
 ;; Author: Distopico <distopico@riseup.net>
-;; Package-Requires: ((emacs "26.1") (flymake "1.2.1") (flymake-quickdef "1.0.0"))
+;; Package-Requires: ((emacs "26.1") (flymake "1.2.1"))
 ;; Keywords: language, tools
 ;; Version: 0.3
 
@@ -35,14 +35,12 @@
 
 ;;; Code:
 
-(require 'flymake-quickdef)
-
 (defgroup flycheck-guile nil
   "GNU Guile Flymake backend."
   :prefix "flymake-guile-"
   :group 'flymake)
 
-(defcustom flymake-guile-guild-binary "guild"
+(defcustom flymake-guile-guild-executable "guild"
   "Name of the Guile `guild' executable."
   :type 'string
   :group 'flymake-guile)
@@ -142,47 +140,122 @@ Also verify if the `STACK-FILE' and the source file are te same."
 		    (- col 1))))
 	  text)))
 
-(flymake-quickdef-backend flymake-guile-backend
-  :pre-let ((guild-exec (executable-find flymake-guile-guild-binary)))
-  :pre-check (unless guild-exec (error "Cannot find guild executable"))
-  :write-type 'file
-  :proc-form (append
-	      (list guild-exec
-		"compile"
-		"-O0")
-	      (flymake-guile--warning-level-args)
-	      (flymake-guile--load-path-args)
-	      flymake-guile-guild-args
-	      (list fmqd-temp-file))
-  :search-regexp (concat
-		  "\\(.*\\)"
-		  flymake-guile--diag-lnum-rx
-		  "\\(.*\\):[[:space:]]?"
-		  "\\(Syntax error:[[:space:]].*\\|.*\\)$")
-  :prep-diagnostic
-  (let* ((stack_file (match-string 1))
-	 (stack_lnum (match-string 2))
-	 (stack_cnum (match-string 3))
-	 (severity (match-string 4))
-	 (stack_msg (match-string 5))
-	 (report (flymake-guile--get-diagnostic
-		  stack_msg
-		  stack_lnum
-		  stack_cnum
-		  stack_file
-		  fmqd-source))
-	 (lnum (car (car report)))
-	 (cnum (cdr (car report)))
-	 (text (cdr report))
-	 (pos (flymake-diag-region fmqd-source lnum cnum))
-	 (beg (car pos))
-	 (end (cdr pos))
-	 (type (cond
-		((string= severity "warning") :warning)
-		((string= severity "In procedure raise-exception") :error)
-		(t :note)))
-	 (msg (string-trim text)))
-    (list fmqd-source beg end type msg)))
+(defvar-local flymake-guile-process nil)
+
+(defun flymake-make-guile-sentinel (report-fn source temp-dir)
+  "Generate a process sentinel reporting to REPORT-FN.
+The argument SOURCE and TEMP-DIR are respectively used to pass
+the buffer containing the source code being checked and the
+temporary director generated for the checking."
+  (lambda (proc _event)
+    (unless (process-live-p proc)
+      (unwind-protect
+	  (if (eq proc (buffer-local-value 'flymake-guile-process source))
+	      (with-current-buffer source
+		(save-restriction
+		  (widen)
+		  (with-current-buffer (process-buffer proc)
+		    (goto-char (point-min))
+		    (save-match-data
+		      (let ((diags nil))
+			(while (search-forward-regexp
+				(concat
+				 "\\(.*\\)"
+				 flymake-guile--diag-lnum-rx
+				 "\\(.*\\):[[:space:]]?"
+				 "\\(Syntax error:[[:space:]].*\\|.*\\)$")
+				nil t)
+			  (save-match-data
+			    (save-excursion
+			      (let* ((diag-vals
+				      (let* ((stack_file
+					      (match-string 1))
+					     (stack_lnum
+					      (match-string 2))
+					     (stack_cnum
+					      (match-string 3))
+					     (severity
+					      (match-string 4))
+					     (stack_msg
+					      (match-string 5))
+					     (report
+					      (flymake-guile--get-diagnostic
+					       stack_msg
+					       stack_lnum
+					       stack_cnum
+					       stack_file
+					       source))
+					     (lnum (caar report))
+					     (cnum (cdar report))
+					     (text (cdr report))
+					     (pos
+					      (flymake-diag-region
+					       source
+					       lnum
+					       cnum))
+					     (beg (car pos))
+					     (end (cdr pos))
+					     (type
+					      (cond
+					       ((string= severity "warning") :warning)
+					       ((string= severity "In procedure raise-exception")
+						:error)
+					       (t :note)))
+					     (msg (string-trim text)))
+					(list source beg end type msg)))
+				     (diag-beg (nth 1 diag-vals))
+				     (diag-end (nth 2 diag-vals))
+				     (diag-type (nth 3 diag-vals)))
+				(if (and
+				     (integer-or-marker-p diag-beg)
+				     (integer-or-marker-p diag-end))
+				    (when diag-type
+				      (push (apply #'flymake-make-diagnostic diag-vals)
+					    diags))
+				  (with-current-buffer source
+				    (flymake-log
+				     :error
+				     "Got invalid buffer position %s or %s in %s"
+				     diag-beg
+				     diag-end
+				     proc)))))))
+			(funcall
+			 report-fn
+			 (nreverse
+			  diags)))))))
+	    (flymake-log :warning
+			 "Canceling obsolete check %s"
+			 proc))
+	(delete-directory temp-dir t)
+	(kill-buffer (process-buffer proc))))))
+
+(defun flymake-guile-backend (report-fn &rest _args)
+  "Flymake backend for Scheme buffers using GNU Guile.
+For the interpretation of REPORT-FN, consult the Info
+node `(flymake) Backend functions'."
+  (let* ((guild-exec (or (executable-find flymake-guile-guild-executable)
+			 (error "Cannot find guild executable")))
+	 (temp-dir (make-temp-file "flymake-guile-" t))
+	 (temp-file (expand-file-name
+		     (file-name-nondirectory (or (buffer-file-name) (buffer-name)))
+		     temp-dir)))
+    (when (process-live-p flymake-guile-process)
+      (kill-process flymake-guile-process))
+    (save-restriction
+      (widen)
+      (write-region nil nil temp-file nil 'silent)
+      (setq flymake-guile-process
+	    (make-process
+	     :name "flymake-guile-backend-flymake"
+	     :noquery t :connection-type 'pipe
+	     :buffer (generate-new-buffer " *flymake-guile-backend-flymake*")
+	     :sentinel (flymake-make-guile-sentinel report-fn (current-buffer) temp-dir)
+	     :command
+	     (append (list guild-exec "compile" "-O0")
+		     (flymake-guile--warning-level-args)
+		     (flymake-guile--load-path-args)
+		     flymake-guile-guild-args
+		     (list temp-file)))))))
 
 ;;;###autoload
 (defun flymake-guile ()