From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: [ELPA] some tex-related packages Date: Fri, 18 Oct 2024 11:00:26 +0000 Message-ID: <87plnxisl1.fsf@posteo.net> References: <87msol9emj.fsf@posteo.net> <87cypgn8oc.fsf@posteo.net> <87ikyrdw24.fsf@posteo.net> <87wmn7ce9f.fsf@posteo.net> <87h6dryhil.fsf@posteo.net> <877cenyh7t.fsf@posteo.net> <87wmmmwyaz.fsf@posteo.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="13187"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Arash Esbati , emacs-devel@gnu.org To: Paul Nelson Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Oct 18 13:01:27 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1t1kjW-0003Ct-QG for ged-emacs-devel@m.gmane-mx.org; Fri, 18 Oct 2024 13:01:26 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t1kij-0005ZZ-21; Fri, 18 Oct 2024 07:00:37 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t1kih-0005Z3-0E for emacs-devel@gnu.org; Fri, 18 Oct 2024 07:00:35 -0400 Original-Received: from mout02.posteo.de ([185.67.36.66]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t1kic-0002es-It for emacs-devel@gnu.org; Fri, 18 Oct 2024 07:00:34 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id F0A19240101 for ; Fri, 18 Oct 2024 13:00:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1729249227; bh=IcYISmZueUXPxbwHYK7b3iGBw/cfeYdV7GM4djJnjn0=; h=From:To:Cc:Subject:Autocrypt:OpenPGP:Date:Message-ID:MIME-Version: Content-Type:From; b=FUc3rXUF6Rcfr5eUnk+Ir5z9si1UychoShluT/c6Fko3M66FCM8wJw3JDFeAqW/mX CNy+xaol6Jv+/P2bEhhcM6/p+gZQw7iO9JsdeZzi954eqtKxVQntX1NaqLYaxZmzTW fBoRwVuMlGH11V1Qpw/iS5bZMwYiLI7eWoWPSmlVe2ujHWndbJjpYDEHX0ysaMRg+r rkuT7GZabZr2RENdGNNt1toT5zlGBuF/Nmvi2lnHf6gALjQ4TqutmDm0UsmolfAy3P x2aOrkVti1XS2tb5h0nErOvbSDbG2gJwQb9+krKoZEs4sOBiwb/0k6/dFhulRSSy6m FpDbBzLLdrfkQ== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4XVMDq0brCz6twH; Fri, 18 Oct 2024 13:00:26 +0200 (CEST) In-Reply-To: (Paul Nelson's message of "Sat, 12 Oct 2024 17:30:36 +0200") Autocrypt: addr=philipk@posteo.net; keydata= mDMEZBBQQhYJKwYBBAHaRw8BAQdAHJuofBrfqFh12uQu0Yi7mrl525F28eTmwUDflFNmdui0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiWBBMWCAA+FiEEDg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwMFCQHhM4AFCwkI BwIGFQoJCAsCBBYCAwECHgECF4AACgkQ8xYDWXahwulikAEA77hloUiSrXgFkUVJhlKBpLCHUjA0 mWZ9j9w5d08+jVwBAK6c4iGP7j+/PhbkxaEKa4V3MzIl7zJkcNNjHCXmvFcEuDgEZBBQQhIKKwYB BAGXVQEFAQEHQI5NLiLRjZy3OfSt1dhCmFyn+fN/QKELUYQetiaoe+MMAwEIB4h+BBgWCAAmFiEE Dg7HY17ghYlni8XN8xYDWXahwukFAmQQUEICGwwFCQHhM4AACgkQ8xYDWXahwukm+wEA8cml4JpK NeAu65rg+auKrPOP6TP/4YWRCTIvuYDm0joBALw98AMz7/qMHvSCeU/hw9PL6u6R2EScxtpKnWof z4oM OpenPGP: id=philipk@posteo.net; url="https://keys.openpgp.org/vks/v1/by-email/philipk@posteo.net"; preference=signencrypt Received-SPF: pass client-ip=185.67.36.66; envelope-from=philipk@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:324653 Archived-At: --=-=-= Content-Type: text/plain Paul Nelson writes: > Hi Philip, > >> 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: > > I suspect your backlog remains much the same as a few months ago, but > figured I'd send a friendly nudge to see if you might have the chance > to review any further packages from the list, i.e., one of > > https://github.com/ultronozm/tex-parens.el Here are my comments/questions for this package: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=tex-parens.el.diff diff --git a/tex-parens.el b/tex-parens.el index fb93734..a2b4c5d 100644 --- a/tex-parens.el +++ b/tex-parens.el @@ -40,8 +40,10 @@ ;;; Code: +(eval-when-compile (require 'rx)) + (defgroup tex-parens () - "Like lisp.el but for tex." + "Like lisp.el but for tex." ;I'd try to elaborate what this means :group 'tex) (defun tex-parens--beginning-of-defun () @@ -64,7 +66,7 @@ Here `defun' means top-level environment." ("\\lfloor" . "\\rfloor") ("\\lceil" . "\\rceil")) "Left/right pairs of delimiters." - :type '(repeat (cons string string))) + :type '(repeat (cons (string :tag "Left pair") (string :tag "Right pair")))) (defcustom tex-parens-solo-delimiters '("|" "\\|" "\\vert" "\\Vert") @@ -98,11 +100,11 @@ Here `defun' means top-level environment." "Maximum length of a delimiter. This is comfortably larger than `\\biggl\\langle' and `\\begin{proposition}', for example." - :type 'integer) + :type 'natnum) (defun tex-parens--reduce-append (func list1 list2) "List consisting of FUNC applied to pairs from LIST1 and LIST2." - (seq-reduce #'append + (seq-reduce #'append ;couldn't you use `mapcan'? (mapcar (lambda (item1) (mapcar (lambda (item2) (funcall func item1 item2)) @@ -166,19 +168,19 @@ form delimiters which are visibly `left'/`opening' or ;; If AUCTeX 14.0.5+ is installed, then we make some of the ;; navigation commands automatically open previews and folds. (when (boundp 'preview-auto-reveal-commands) - (dolist (func '(tex-parens-down-list - tex-parens-backward-down-list)) + (dolist (func (list #'tex-parens-down-list + #'tex-parens-backward-down-list)) (add-to-list 'preview-auto-reveal-commands func))) (when (boundp 'TeX-fold-auto-reveal-commands) - (dolist (func '(tex-parens-down-list - tex-parens-backward-down-list - tex-parens-up-list - tex-parens-backward-up-list - tex-parens-forward-list - tex-parens-backward-list - tex-parens-forward-sexp - tex-parens-backward-sexp)) + (dolist (func (list #'tex-parens-down-list + #'tex-parens-backward-down-list + #'tex-parens-up-list + #'tex-parens-backward-up-list + #'tex-parens-forward-list + #'tex-parens-backward-list + #'tex-parens-forward-sexp + #'tex-parens-backward-sexp)) (add-to-list 'TeX-fold-auto-reveal-commands func))) (setq-local tex-parens--saved-beginning-of-defun-function @@ -190,8 +192,8 @@ form delimiters which are visibly `left'/`opening' or (setq tex-parens--pairs (tex-parens--generate-pairs)) (setq tex-parens--pairs-swap (mapcar (lambda (x) (cons (cdr x) (car x))) tex-parens--pairs)) - (setq tex-parens--delims (append (mapcar #'car tex-parens--pairs) - (mapcar #'cdr tex-parens--pairs))) + (setq tex-parens--delims (nconc (mapcar #'car tex-parens--pairs) + (mapcar #'cdr tex-parens--pairs))) (setq tex-parens--regexp (concat @@ -244,23 +246,22 @@ Tex Parens mode is a minor mode in which lisp/sexp/defun-based commands are adapted to tex environments and math delimiters. The affected commands include, for instance, `forward-sexp', `forward-list' and `beginning-of-defun'." - :lighter nil - :keymap (let ((map (make-sparse-keymap))) + :keymap (let ((map (make-sparse-keymap))) ;why not define it in a + ;separate variable? it + ;would also avoid overlong + ;lines. (define-key map [remap forward-sexp] #'tex-parens-forward-sexp) (define-key map [remap backward-sexp] #'tex-parens-backward-sexp) (define-key map [remap forward-list] #'tex-parens-forward-list) (define-key map [remap backward-list] #'tex-parens-backward-list) - (define-key map [remap backward-up-list] - #'tex-parens-backward-up-list) + (define-key map [remap backward-up-list] #'tex-parens-backward-up-list) (define-key map [remap up-list] #'tex-parens-up-list) (define-key map [remap down-list] #'tex-parens-down-list) (define-key map [remap delete-pair] #'tex-parens-delete-pair) (define-key map [remap mark-sexp] #'tex-parens-mark-sexp) (define-key map [remap kill-sexp] #'tex-parens-kill-sexp) - (define-key map [remap backward-kill-sexp] - #'tex-parens-backward-kill-sexp) - (define-key map [remap transpose-sexps] - #'tex-parens-transpose-sexps) + (define-key map [remap backward-kill-sexp] #'tex-parens-backward-kill-sexp) + (define-key map [remap transpose-sexps] #'tex-parens-transpose-sexps) (define-key map [remap raise-sexp] #'tex-parens-raise-sexp) map) (cond @@ -401,7 +402,7 @@ delimiter. Otherwise, return nil." (and (stringp delim) (or - (and (or + (and (or ;I'd use `rx' here (string-match "\\\\begin{\\([^}]+\\)}\\[[^]]+\\]" delim) (string-match "\\\\begin{\\([^}]+\\)}" delim)) (let ((type (match-string 1 delim))) @@ -506,6 +507,7 @@ Search up to BOUND." (when tex-parens--debug (message "Unmatched delimiters: %s" (car stack)))))) +;; doesn't this function share a lot of code with the previous function? (defun tex-parens--backward-list-1 (&optional bound) "Move backward across one balanced group. Search up to BOUND." @@ -1096,38 +1098,43 @@ This regexp matches the start of various math environments, keeping a couple of characters before the primary match to leave space for Avy." :type 'regexp) +(declare-function avy-jump "avy" (regex &rest _)) (defun tex-parens-avy-jump-to-math () "Jump inside a math expression using Avy. This function uses `tex-parens-avy-regexp' to identify potential math expressions, then jumps to the selected one and moves point inside the expression." (interactive) - (if (fboundp 'avy-jump) - (avy-jump tex-parens-avy-regexp - :action (lambda (pos) - (goto-char pos) - (forward-char 2) - (tex-parens-down-list) - ;; For preview-auto-reveal: - (setq this-command #'tex-parens-down-list))) - (user-error "Avy is not available. Please install and load it to use this function"))) + (unless (and (fboundp 'avy-jump) + (require 'avy nil t) + (fboundp 'avy-jump)) + (error "Avy is not available. Please install it to use this function")) + (avy-jump tex-parens-avy-regexp + :action (lambda (pos) + (goto-char pos) + (forward-char 2) + (tex-parens-down-list) + ;; For preview-auto-reveal: + (setq this-command #'tex-parens-down-list)))) (defun tex-parens-avy-copy-math () "Copy a math expression selected using Avy. This function uses `tex-parens-avy-regexp' to identify potential math expressions, then copies the selected one to the kill ring." (interactive) - (if (fboundp 'avy-jump) - (avy-jump tex-parens-avy-regexp - :action (lambda (pos) - (let ((beg (+ pos 2)) - (end (save-excursion - (goto-char (+ pos 2)) - (tex-parens-forward-list) - (point)))) - (copy-region-as-kill beg end) - (message "Math expression copied")))) - (user-error "Avy is not available. Please install and load it to use this function"))) + (unless (and (fboundp 'avy-jump) + (require 'avy nil t) + (fboundp 'avy-jump)) + (error "Avy is not available. Please install it to use this function")) + (avy-jump tex-parens-avy-regexp + :action (lambda (pos) + (let ((beg (+ pos 2)) + (end (save-excursion + (goto-char (+ pos 2)) + (tex-parens-forward-list) + (point)))) + (copy-region-as-kill beg end) + (message "Math expression copied"))))) (provide 'tex-parens) ;;; tex-parens.el ends here --=-=-= Content-Type: text/plain > https://github.com/ultronozm/auctex-label-numbers.el No special comments here, perhaps check if things like user option types could be made more specific. > https://github.com/ultronozm/preview-auto.el Just a few comments here: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=preview-auto.el.diff diff --git a/preview-auto.el b/preview-auto.el index 7614b4f..da184a2 100644 --- a/preview-auto.el +++ b/preview-auto.el @@ -39,6 +39,7 @@ ;;; Code: +(eval-when-compile (require 'cl-lib)) (require 'latex) (require 'preview) @@ -90,7 +91,6 @@ Should work in AUCTeX `LaTeX-mode' buffers. Implemented using (memq math-face face)))) (texmathp))) - (defun preview-auto--generate-rules () "Return list of rules for identifying math environments." (let* ((basic-rules @@ -120,7 +120,6 @@ is valid.") (defvar-local preview-auto--begin-re nil "Regular expression for identifying the beginning of a math environment.") - (defun preview-auto--check-default () "Default predicate for checking whether to consider a delimiter. Returns non-nil if either @@ -543,6 +542,7 @@ cancel the preview, so that the preview is not misplaced." (remove-hook 'after-change-functions #'preview-auto--after-change t) (remove-hook 'post-command-hook #'preview-auto--post-command t)))) +;;;###autoload (defun preview-auto-setup () "Hook function for installing bind and menu item." (remove-hook 'LaTeX-mode-hook #'preview-auto-setup) @@ -552,6 +552,9 @@ cancel the preview, so that the preview is not misplaced." ["automatically" preview-auto-mode] "(or toggle) at point")) +;; Please don't modify hooks on the top-level! Even if it removes +;; itself, it is better to advise the user to call this or by hand or +;; copy/adjust the code as they see fit. (add-hook 'LaTeX-mode-hook #'preview-auto-setup) (provide 'preview-auto) --=-=-= Content-Type: text/plain > https://github.com/ultronozm/tex-item.el Here nothing noteworthy to say from my side. > https://github.com/ultronozm/preview-tailor.el Again, nothing special to say. I just think you should include cl-lib and seq (or just one of the two). > (I recall tex-parens being raised as something that might be usefully > included in Emacs, so maybe that would be a natural next target?) > > Thanks, best, > > Paul -- Philip Kaludercic on siskin --=-=-=--