Re: [ELPA] some tex-related packages

[Philip Kaludercic <philipk@posteo.net>]

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

Paul Nelson <ultrono@gmail.com> writes:

> Hi Philip,
>
> OK, thanks.  I adjusted the copyrights as suggested, and bumped the
> version numbers -- I don't see the packages listed yet, but assume
> this will happen automatically after some time.

Right, a cron job synchronises the repositories on a regular basis, and
should pick up your commit soon enough.

> Sorry to hear again about the backlog.  Whenever you get the chance,
> I'd very much welcome any feedback, even on just one or two of the
> packages, since I'm sure I'd learn something from it.

I've taken a look at one package, and am adding the comments to this
message, and hope to do the same for the others, step by step:


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

diff --git a/auctex-cont-latexmk.el b/auctex-cont-latexmk.el
index 442972475d..9bdcf465c7 100644
--- a/auctex-cont-latexmk.el
+++ b/auctex-cont-latexmk.el
@@ -63,10 +63,7 @@
   (let ((error-alist
          (append TeX-error-description-list
                  TeX-error-description-list-local)))
-    (catch 'found
-      (dolist (error error-alist)
-        (when (string-match (car error) message)
-          (throw 'found (cdr error)))))))
+    (alist-get message error-alist nil nil #'string-match-p)))
 
 (defun auctex-cont-latexmk-help-at-point ()
   "Display the AUCTeX help for the error at point."
@@ -81,48 +78,47 @@ The arguments are as in `TeX-error-list'.  Return either nil or a
 triple (ERROR-P DESCRIPTION (BEG . END)), where ERROR-P is non-nil if it
 is an error rather than a warning."
   (or
-   (and
-    (not ignore)
-    (stringp file)
-    (or (not bad-box) TeX-debug-bad-boxes)
-    (when-let
-        ((region
-          (save-restriction
-            (widen)
-            (cond
-             ((file-equal-p
-               file
-               (or buffer-file-name (buffer-file-name (buffer-base-buffer))))
-              (when line
-                (if (eq type 'error)
+   (and-let*
+       (((not ignore))
+        ((stringp file))
+        ((or (not bad-box) TeX-debug-bad-boxes))
+        (region
+         (save-restriction
+           (widen)
+           (cond
+            ((file-equal-p
+              file
+              (or buffer-file-name (buffer-file-name (buffer-base-buffer))))
+             (when line
+               (if (eq type 'error)
+                   (save-excursion
+                     (goto-char (point-min))
+                     (forward-line (+ line offset -1))
+                     (unless (string= search-string " ")
+                       (search-forward search-string nil t)
+                       (cons (point) (1+ (point)))))
+                 (flymake-diag-region (current-buffer) (+ line offset)))))
+            ((file-equal-p file (TeX-master-output-file "aux"))
+             (and auctex-cont-latexmk-report-multiple-labels
+                  (string-match-p "multiply defined" message)
+                  (not (eq type 'error))
+                  (let* ((label (progn
+                                  (string-match "`\\(.*\\)'" message)
+                                  (match-string 1 message)))
+                         (label-re
+                          (concat "\\\\label\\(?:\\[[^]]+\\]\\)?{"
+                                  (regexp-quote label) "}")))
                     (save-excursion
                       (goto-char (point-min))
-                      (forward-line (+ line offset -1))
-                      (unless (string= search-string " ")
-                        (search-forward search-string nil t)
-                        (cons (point) (1+ (point)))))
-                  (flymake-diag-region (current-buffer) (+ line offset)))))
-             ((file-equal-p file (TeX-master-output-file "aux"))
-              (and auctex-cont-latexmk-report-multiple-labels
-                   (string-match-p "multiply defined" message)
-                   (not (eq type 'error))
-                   (let* ((label (progn
-                                   (string-match "`\\(.*\\)'" message)
-                                   (match-string 1 message)))
-                          (label-re
-                           (concat "\\\\label\\(?:\\[[^]]+\\]\\)?{"
-                                   (regexp-quote label) "}")))
-                     (save-excursion
-                       (goto-char (point-min))
-                       (when (re-search-forward label-re nil t)
-                         ;; Return the full line so the diagnostic is
-                         ;; not covered by preview overlays when
-                         ;; \\label appears after \\begin{equation}.
-                         (cons (line-beginning-position)
-                               (line-end-position)))))))))))
-      (list (eq type 'error)
-            (replace-regexp-in-string "\n" "" message)
-            region)))
+                      (when (re-search-forward label-re nil t)
+                        ;; Return the full line so the diagnostic is
+                        ;; not covered by preview overlays when
+                        ;; \\label appears after \\begin{equation}.
+                        (cons (line-beginning-position)
+                              (line-end-position)))))))))))
+     (list (eq type 'error)
+           (replace-regexp-in-string "\n" "" message)
+           region))
    ;; Put errors without file or line at bottom of buffer.
    (when (eq type 'error)
      (list t
@@ -133,9 +129,11 @@ is an error rather than a warning."
   "Format the current log buffer by joining lines suitably.
 Adapted from `TeX-format-filter'."
   (goto-char (point-max))
-  (while (> (point) (point-min))
+  (while (not (bobp))
     (end-of-line 0)
-    (when (and (memq (- (point) (line-beginning-position)) '(79 80))
+    (when (and (<= 79 (current-column) 80)
+               ;; I was imagining if this could be replaced by a clever regular expression like
+               ;; (not (looking-back (rx "." point (syntax word)) (line-beginning-position)))
                (not (memq (char-after (1+ (point))) '(?\n ?\()))
                (not (and (eq (char-before) ?.)
                          (char-after (1+ (point)))
@@ -155,10 +153,11 @@ Adapted from `TeX-format-filter'."
   "Process log file for current LaTeX document.
 Return a list of triples as in the docstring of
 `auctex-cont-latexmk-process-item'."
-  (delq nil
-        (mapcar (lambda (item)
-                  (apply #'auctex-cont-latexmk-process-item item))
-                (auctex-cont-latexmk--error-list (TeX-master-output-file "log")))))
+  (mapcan
+   (lambda (item)
+     (and-let* ((item (apply #'auctex-cont-latexmk-process-item item)))
+       (list item)))
+   (auctex-cont-latexmk--error-list (TeX-master-output-file "log"))))
 
 (defvar-local auctex-cont-latexmk--report-fn nil
   "Function provided by Flymake for reporting diagnostics.")
@@ -204,7 +203,7 @@ Flymake report function to propagate to indirect buffers."
 ;;; Continuous Compilation
 
 (defcustom auctex-cont-latexmk-command
-  '("latexmk -pvc -shell-escape -pdf -view=none -e "
+  '("latexmk -pvc -shell-escape -pdf -view=none -e " ;is it safe to have "shell-escape" in by default?
     ("$pdflatex=q/pdflatex %O -synctex=1 -interaction=nonstopmode %S/"))
   "Command to compile LaTeX documents.
 This is a list consisting of strings or lists of strings.  It is
@@ -218,6 +217,10 @@ file."
 (defun auctex-cont-latexmk--compilation-command ()
   "Return the command used to compile the current LaTeX document."
   (let ((quote
+         ;; I am surprised to see that this is necessary, but I don't
+         ;; know the background.  does `shell-quote-argument' quote
+         ;; incorrectly?  A comment explaining what is going on would
+         ;; be nice.
          (if (memq system-type '(ms-dos windows-nt))
              "\""
            "'")))
@@ -267,11 +270,9 @@ either in a watching state or has not updated recently."
      (when-let ((buf auctex-cont-latexmk--compilation-buffer))
        (with-current-buffer buf
          (or
-          (progn
-            (goto-char (point-max))
-            (forward-line -1)
-            (equal (buffer-substring (point) (line-end-position))
-                   auctex-cont-latexmk--watching-str))
+          (search-forward-regexp
+           (rx (literal auctex-cont-latexmk--watching-str) (? ?\n) eos)
+           nil t)
           (and (or
                 auctex-cont-latexmk--last-update-time
                 (time-less-p (time-subtract (current-time)
@@ -314,7 +315,7 @@ empty, except when NOKILL is non-nil."
         (let ((process (get-buffer-process comp-buf)))
           (when (process-live-p process)
             (interrupt-process process)
-            (sit-for 0.1)
+            (sit-for 0.1)               ;isn't this racy?  or is that not a problem?
             (delete-process process))
           (unless nokill
             (kill-buffer comp-buf)))))))
@@ -390,7 +391,7 @@ Saved and restored by `auctex-cont-latexmk-toggle'.")
 (defcustom auctex-cont-latexmk-retained-flymake-backends
   '(eglot-flymake-backend)
   "Flymake backends to retain when enabling `auctex-cont-latexmk-mode'."
-  :type 'boolean)
+  :type 'boolean)                       ;I don't think the type is right
 
 (defun auctex-cont-latexmk-turn-on ()
   "Enable `auctex-cont-latexmk-mode' and set up Flymake."
@@ -425,7 +426,7 @@ Saved and restored by `auctex-cont-latexmk-toggle'.")
 (defun auctex-cont-latexmk-toggle ()
   "Toggle `auctex-cont-latexmk-mode' and its Flymake backend."
   (interactive)
-  (cond (auctex-cont-latexmk-mode
+  (cond (auctex-cont-latexmk-mode       ;isn't this more of a `if'-place?
          (auctex-cont-latexmk-turn-off))
         (t
          (auctex-cont-latexmk-turn-on))))

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


> Thanks, best,
>
> Paul

-- 
	Philip Kaludercic on peregrine