Re: [ELPA] Proposing to add express to ELPA

[Philip Kaludercic <philipk@posteo.net>]

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

Yuan Fu <casouri@gmail.com> writes:

> Hi all,

Hi,

> Since Emacs 29 is now released, I’d like to propose adding expreg to
> ELPA. Expreg can be considered a lite version of expand-region. The
> notable difference is its use of tree-sitter for language-specific
> expansions. I also took the liberty to do things differently than
> expand-region, eg, expreg uses a smaller number of expanders [1]; it
> is easier to debug when the expansion isn’t what you expected; and it
> only provides two functions for expansion and contraction, and one
> variable for adding/removing expanders—no transient maps and other
> “smart” features, nor different variables to set for each major mode.
>
> The obvious downsides is that, of course, it’s pretty useless on
> anything other than lisp if you don’t have tree-sitter grammars and
> major mode installed. You can use it in a non-tree-sitter major mode,
> as long the tree-sitter grammar exists. You only need to create a
> parser and expreg will automatically use the parser [2].
>
> I’ve been using it for months and ironed out all sorts of edge-cases, and can recommend it for daily usage.

this looks nice!  I have a few comments that might be interesting:


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

diff --git a/expreg.el b/expreg.el
index 05459af..2147ce8 100644
--- a/expreg.el
+++ b/expreg.el
@@ -1,6 +1,6 @@
 ;;; expreg.el --- Simple expand region  -*- lexical-binding: t; -*-
 
-;; Copyright (C) 2022 Free Software Foundation, Inc.
+;; Copyright (C) 2022, 2023 Free Software Foundation, Inc.
 ;;
 ;; Author: Yuan Fu <casouri@gmail.com>
 ;; Maintainer: Yuan Fu <casouri@gmail.com>
@@ -79,8 +79,8 @@
 (require 'subword)
 (require 'treesit)
 (eval-when-compile
-  (require 'cl-lib)
-  (require 'seq))
+  (require 'cl-lib))
+(require 'seq)
 
 ;;; Cutom options and variables
 
@@ -102,9 +102,9 @@ scan-error, like end-of-buffer, or unbalanced parentheses, etc.")
 
 (defun expreg--sort-regions (regions)
   "Sort REGIONS by their span."
-  (cl-sort regions (lambda (a b)
-                     (< (- (cddr a) (cadr a))
-                        (- (cddr b) (cadr b))))))
+  (sort regions (lambda (a b)
+                  (< (- (cddr a) (cadr a))
+                     (- (cddr b) (cadr b))))))
 
 (defvar expreg--validation-white-list '(list-at-point)
   "Regions produced by functions in this list skips filtering.")
@@ -166,12 +166,11 @@ ORIG is the current position. Each region is (BEG . END)."
     ;; OTOH, if there are regions that ends at ORIG, filter out
     ;; regions that starts AFTER ORIGN, eg, special cases in
     ;; ‘expreg--list-at-point’.
-    (setq regions (cl-remove-if
-                   (lambda (region)
-                     (and orig-at-end-of-something
-                          (> (cadr region) orig)))
-                   regions))
-    regions))
+    (cl-remove-if
+     (lambda (region)
+       (and orig-at-end-of-something
+            (> (cadr region) orig)))
+     regions)))
 
 ;;; Syntax-ppss shorthands
 
@@ -199,7 +198,7 @@ POS defaults to point."
 ;;; Expand/contract
 
 (defvar-local expreg--verbose nil
-  "If t, print debugging information.")
+  "Non-nil means print debugging information.")
 
 (defvar-local expreg--next-regions nil
   "The regions we are going to expand to.
@@ -209,12 +208,13 @@ This should be a list of (BEG . END).")
   "The regions we’ve expanded past.
 This should be a list of (BEG . END).")
 
+;;;###autoload
 (defun expreg-expand ()
   "Expand region."
   (interactive)
   ;; Checking for last-command isn’t strictly necessary, but nice to
   ;; have.
-  (when (not (and (region-active-p)
+  (when (not (and (use-region-p)
                   (eq (region-beginning)
                       (cadr (car expreg--prev-regions)))
                   (eq (region-end)
@@ -236,7 +236,7 @@ This should be a list of (BEG . END).")
 
   ;; Go past all the regions that are smaller than the current region,
   ;; if region is active.
-  (when (region-active-p)
+  (when (use-region-p)
     (while (and expreg--next-regions
                 (let ((beg (cadr (car expreg--next-regions)))
                       (end (cddr (car expreg--next-regions))))
@@ -261,8 +261,8 @@ This should be a list of (BEG . END).")
 (defun expreg-contract ()
   "Contract region."
   (interactive)
-  (when (and (region-active-p)
-             (> (length expreg--prev-regions) 1))
+  (when (and (use-region-p)
+             (length> expreg--prev-regions 1))
 
     (push (pop expreg--prev-regions) expreg--next-regions)
     (set-mark (cddr (car expreg--prev-regions)))
@@ -548,7 +548,7 @@ current string/comment and get lists inside."
 (defun expreg--comment ()
   "Return a list of regions containing comment."
   (let ((orig (point))
-        (beg (point))
+        (beg (point))			;perhaps use narrowing?
         (end (point))
         result forward-succeeded trailing-comment-p)
 

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


> You can find the repository here: https://github.com/casouri/expreg
> And I attached a patch for ELPA. It’s been awhile since I last made a
> patch for ELPA, I hope I did it right.

Looks OK.

> [1] Default expanders include: expreg--subword expreg--word expreg--list expreg--string expreg--treesit expreg--comment expreg--paragraph
>
> [2] Something like (add-hook 'xxx-mode-hook (lambda () (treesit-parser-create 'xxx)))
>
> PS. I find it amusing that, among the total 632 LOC, only 17 are
> responsible for the tree-sitter support, the main purpose of this
> package; all the rest are code dealing with correctly expanding lists,
> strings and comments with syntax-ppss.
>
> Thanks,
> Yuan
>
> From 7e201deb71f324e22d31331c06cf3999a105668b Mon Sep 17 00:00:00 2001
> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 31 Jul 2023 11:14:04 -0700
> Subject: [PATCH] * elpa-packages (expreg): New package.
>
> ---
>  elpa-packages | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/elpa-packages b/elpa-packages
> index 48a0ada..e1470d5 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -292,6 +292,7 @@
>    :doc "doc/ess.texi")
>   (excorporate		:url nil)
>   (expand-region		:url "https://github.com/magnars/expand-region.el")
> + (expreg                :url "https://github.com/casouri/expreg.git")
>   (external-completion   :core "lisp/external-completion.el")
>   (exwm			:url "https://github.com/ch11ng/exwm.git")
>   (f90-interface-browser :url nil) ;; Was "https://github.com/wence-/f90-iface"