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

[Philip Kaludercic]

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:

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 ()