all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [NonGNU ELPA] New package: latex-table-wizard
@ 2022-12-16 17:17 Enrico Flor
  2022-12-16 19:46 ` Philip Kaludercic
  0 siblings, 1 reply; 11+ messages in thread
From: Enrico Flor @ 2022-12-16 17:17 UTC (permalink / raw)
  To: emacs-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 809 bytes --]

I would like to submit latex-table-wizard to NonGNU ELPA.  This package
depends on AucTeX and on transient, and provides a minor mode with a
series of commands to navigate and edit complicated LaTeX table-like
environments (the standard ones, but the user can define additional
ones).

With a transient UI, this package allows you to:

+ navigate "logically" (that is, move by cells)

+ insert or kill rows or column

+ move arbitrary cells or groups of cells around

+ align the table in different ways (however alignment is not needed for
  the functionalities above)

These commands are not fooled by the presence of embedded tables or
other complications (for example: while editing a larger table, a buffer
substring like:

    & ... \makecell{ a & b \\ c & d} ... &

is still parsed as a single cell).


[-- Attachment #1.1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

[-- Attachment #1.2: latex-table-wizard patch --]
[-- Type: text/x-patch, Size: 1350 bytes --]

From 27f25c72ed8e0e3e81cfc4f996f8c03c9c0155fe Mon Sep 17 00:00:00 2001
From: Enrico Flor <nericoflor@gmail.com>
Date: Fri, 16 Dec 2022 10:58:55 -0500
Subject: [PATCH] Add latex-table-wizard

---
 elpa-packages | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/elpa-packages b/elpa-packages
index 8254411cb2..90356989cb 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -78,7 +78,7 @@
   )

  (cdlatex		:url "https://github.com/cdominik/cdlatex")
-
+
  (cider			:url "https://github.com/clojure-emacs/cider"
   :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
   :news "CHANGELOG.md")
@@ -117,7 +117,7 @@
   :news "changelog.rst")

  (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
-
+
  (dracula-theme		:url "https://github.com/dracula/emacs"
   :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))

@@ -434,6 +434,12 @@
  (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
   :ignored-files ("doc" "test" "Cask" "Makefile"))

+ (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
+  :readme "readme.org"
+  :doc "latex-table-wizard.texi"
+  :news "NEWS"
+  :ignored-files ("COPYING"))
+
  (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
   :readme "README.md")

--
2.38.1


[-- Attachment #1.3: publickey - enrico@eflor.net - ffbd1445.asc --]
[-- Type: application/pgp-keys, Size: 697 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 17:17 [NonGNU ELPA] New package: latex-table-wizard Enrico Flor
@ 2022-12-16 19:46 ` Philip Kaludercic
  2022-12-16 21:29   ` Enrico Flor
  2022-12-17 14:17   ` Philip Kaludercic
  0 siblings, 2 replies; 11+ messages in thread
From: Philip Kaludercic @ 2022-12-16 19:46 UTC (permalink / raw)
  To: Enrico Flor; +Cc: emacs-devel

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

Enrico Flor <enrico@eflor.net> writes:

> I would like to submit latex-table-wizard to NonGNU ELPA.  This package
> depends on AucTeX and on transient, and provides a minor mode with a
> series of commands to navigate and edit complicated LaTeX table-like
> environments (the standard ones, but the user can define additional
> ones).
>
> With a transient UI, this package allows you to:
>
> + navigate "logically" (that is, move by cells)
>
> + insert or kill rows or column
>
> + move arbitrary cells or groups of cells around
>
> + align the table in different ways (however alignment is not needed for
>   the functionalities above)
>
> These commands are not fooled by the presence of embedded tables or
> other complications (for example: while editing a larger table, a buffer
> substring like:
>
>     & ... \makecell{ a & b \\ c & d} ... &
>
> is still parsed as a single cell).
>
> From 27f25c72ed8e0e3e81cfc4f996f8c03c9c0155fe Mon Sep 17 00:00:00 2001
> From: Enrico Flor <nericoflor@gmail.com>
> Date: Fri, 16 Dec 2022 10:58:55 -0500
> Subject: [PATCH] Add latex-table-wizard
>
> ---
>  elpa-packages | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/elpa-packages b/elpa-packages
> index 8254411cb2..90356989cb 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -78,7 +78,7 @@
>    )
>
>   (cdlatex		:url "https://github.com/cdominik/cdlatex")
> -
> +
>   (cider			:url "https://github.com/clojure-emacs/cider"
>    :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
>    :news "CHANGELOG.md")
> @@ -117,7 +117,7 @@
>    :news "changelog.rst")
>
>   (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
> -
> +
>   (dracula-theme		:url "https://github.com/dracula/emacs"
>    :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))
>
> @@ -434,6 +434,12 @@
>   (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
>    :ignored-files ("doc" "test" "Cask" "Makefile"))
>
> + (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"

Here are a few comments in diff-form, just from reading the code:


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

diff --git a/latex-table-wizard.el b/latex-table-wizard.el
index 0238ae3..0b487af 100644
--- a/latex-table-wizard.el
+++ b/latex-table-wizard.el
@@ -90,7 +90,7 @@
 
 (require 'latex)
 (require 'seq)
-(require 'rx)
+(eval-when-compile (require 'rx))
 (require 'regexp-opt)
 (eval-when-compile (require 'subr-x))
 (require 'transient)
@@ -121,13 +121,11 @@ Capture group 1 matches the name of the macro.")
 
 (defcustom latex-table-wizard-column-delimiters '("&")
   "List of strings that are column delimiters if unescaped."
-  :type '(repeat string)
-  :group 'latex-table-wizard)
+  :type '(repeat string))
 
 (defcustom latex-table-wizard-row-delimiters '("\\\\\\\\")
   "List of strings that are row delimiters if unescaped."
-  :type '(repeat string)
-  :group 'latex-table-wizard)
+  :type '(repeat string))
 
 (defcustom latex-table-wizard-hline-macros '("cline"
                                              "vline"
@@ -139,8 +137,7 @@ Capture group 1 matches the name of the macro.")
 
 Each member of this list is a string that would be between the
 \"\\\" and the arguments."
-  :type '(repeat string)
-  :group 'latex-table-wizard)
+  :type '(repeat string))
 
 (defcustom latex-table-wizard-new-environments-alist nil
   "Alist mapping environment names to property lists.
@@ -167,10 +164,9 @@ of a macro that inserts some horizontal line.  For a macro
   :type '(alist :key-type (string :tag "Name of the environment:")
                 :value-type (plist :key-type symbol
                                    :options (:col :row :lines)
-                                   :value-type (repeat string)))
-
-  :group 'latex-table-wizard)
+                                   :value-type (repeat string))))
 
+;; Why not use `memq'?
 (defmacro latex-table-wizard--or (symbol &rest values)
   "Return non-nil if SYMBOL is `eq' to one of VALUES."
   (let ((bools (mapcar (lambda (value) `(eq ,symbol ,value))
@@ -452,18 +448,15 @@ is a cons cell of the form (P . V), where P is either
 If prop-val2 is nil, it is assumed that TABLE is a list of cells
 that only differ for the property in the car of PROP-VAL1 (in
 other words, that TABLE is either a column or a row)"
-  (if prop-val2
-      (catch 'cell
+  (catch 'cell
+    (if prop-val2
         (dolist (x table)
           (when (and (= (cdr prop-val1) (plist-get x (car prop-val1)))
                      (= (cdr prop-val2) (plist-get x (car prop-val2))))
             (throw 'cell x)))
-        nil)
-    (catch 'cell
       (dolist (x table)
         (when (= (cdr prop-val1) (plist-get x (car prop-val1)))
-          (throw 'cell x)))
-      nil)))
+          (throw 'cell x))))))
 
 (defun latex-table-wizard--sort (table same-line dir)
   "Return a sorted table, column or row given TABLE.
@@ -492,10 +485,10 @@ F, C precedes D and so on; and if DIR is either \\='next\\=' or
          (copy-table (copy-sequence table)))
     (if (not same-line)
         (sort copy-table (lambda (x y)
-                           (let ((rows `(,(plist-get x :row)
-                                         ,(plist-get y :row)))
-                                 (cols `(,(plist-get x :column)
-                                         ,(plist-get y :column))))
+                           (let ((rows (list (plist-get x :row)
+                                             (plist-get y :row)))
+                                 (cols (list (plist-get x :column)
+                                             (plist-get y :column))))
                              (cond ((and vert (apply #'= cols))
                                     (apply #'< rows))
                                    (vert
@@ -536,10 +529,9 @@ beginning of the available portion of the buffer."
         (catch 'found
           (while (re-search-forward regexp nil t)
             (when (<= (match-beginning group) position (match-end group))
-              (throw 'found `(,(match-string-no-properties group)
-                              ,(match-beginning group)
-                              ,(match-end group))))
-            nil))))))
+              (throw 'found (list (match-string-no-properties group)
+                                  (match-beginning group)
+                                  (match-end group))))))))))
 
 \f
 
@@ -732,6 +724,9 @@ If SAME-LINE is non-nil, never leave current column or row."
 X and Y are each a list of the form \\='(B E)\\=', where B and E
 are markers corresponding to the beginning and the end of the
 buffer substring."
+  ;; Instead of assuming the form of X and Y, wouldn't it be better to
+  ;; destruct these and make sure?  Then you could also avoid using
+  ;; `apply'?
   (save-excursion
     (let ((x-string (concat " "
                             (string-trim
@@ -821,7 +816,7 @@ Don't print any message if NO-MESSAGE is non-nil."
              (message "Cell (%s,%s) selected for swapping"
                       (plist-get sel :column)
                       (plist-get sel :row)))
-           (latex-table-wizard--hl-cells `(,sel)))
+           (latex-table-wizard--hl-cells (list sel)))
           ((eq thing 'row)
            (unless no-message
              (message "Row %s selected for swapping"
@@ -1261,44 +1256,10 @@ information about how transient suffixes are defined (that is,
 how the data stored in this variable and in
 `latex-table-wizard-default-keys' contributes to the definition
 of the transient prefix)."
-  :type '(alist :key-type
-                (symbol :tag "Command:"
-                        :options (toggle-truncate-lines
-                                  exchange-point-and-mark
-                                  universal-argument
-                                  transient-quit-all
-                                  undo
-                                  latex-table-wizard-right
-                                  latex-table-wizard-right
-                                  latex-table-wizard-left
-                                  latex-table-wizard-up
-                                  latex-table-wizard-down
-                                  latex-table-wizard-end-of-row
-                                  latex-table-wizard-beginning-of-row
-                                  latex-table-wizard-top
-                                  latex-table-wizard-bottom
-                                  latex-table-wizard-beginning-of-cell
-                                  latex-table-wizard-end-of-cell
-                                  latex-table-wizard-mark-cell
-                                  latex-table-wizard-swap-cell-right
-                                  latex-table-wizard-swap-cell-left
-                                  latex-table-wizard-swap-cell-up
-                                  latex-table-wizard-swap-cell-down
-                                  latex-table-wizard-swap-column-right
-                                  latex-table-wizard-swap-column-left
-                                  latex-table-wizard-swap-row-up
-                                  latex-table-wizard-swap-row-down
-                                  latex-table-wizard-align
-                                  latex-table-wizard-select-column
-                                  latex-table-wizard-select-row
-                                  latex-table-wizard-select-deselect-cell
-                                  latex-table-wizard-deselect-all
-                                  latex-table-wizard-swap
-                                  latex-table-wizard-insert-column
-                                  latex-table-wizard-insert-row
-                                  latex-table-wizard-kill-column
-                                  latex-table-wizard-kill-row))
-                :value-type string)
+  :type `(alist :key-type
+                (symbol :tag "Command:")
+                :value-type string
+                :options ,(mapcar #'car latex-table-wizard-default-keys))
   :group 'latex-table-wizard)
 
 (defun latex-table-wizard--make-suffix (symbol)

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


> +  :readme "readme.org"

Are you sure you want to use your "readme.org" file as the package
"readme"?  The contents will be displayed when a user uses
describe-package to find out more about what latex-table-wizard does,
and in my experience it is better to keep it short instead of presenting
a wall of text, that is usually better kept part of the manual.

> +  :doc "latex-table-wizard.texi"

It also appears you have a .info file in your repository that isn't
needed.  That will be generated by the ELPA server, so you don't need to
track the "binary" file in your repository.

If you _do_ intent to build the manual yourself, you should mention the
.info file here, not the .texi file.

> +  :news "NEWS"
> +  :ignored-files ("COPYING"))

Instead of listing files in the package specification, it would be
better to maintain a .elpaignore file in your own repository.

>   (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
>    :readme "README.md")
>
> --
> 2.38.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 19:46 ` Philip Kaludercic
@ 2022-12-16 21:29   ` Enrico Flor
  2022-12-16 21:37     ` Philip Kaludercic
  2022-12-17 14:17   ` Philip Kaludercic
  1 sibling, 1 reply; 11+ messages in thread
From: Enrico Flor @ 2022-12-16 21:29 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 461 bytes --]

Thank you so much for your comments!  I implemented your many
suggestions wrt. the code.  I must say I didn't use to have all the
backquotes but then I read somewhere that you should prefer

  `(,x ,y)

over

  (list x y)

and so I replaced all the instances of the latter with the former.  I
probably misunderstood the "advice".

I also added the .elpaignore, removed the .info file and added a short
description.txt file to serve as readme. 


[-- Attachment #1.1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

[-- Attachment #1.2: patch --]
[-- Type: text/x-patch, Size: 1323 bytes --]

From c711b1b314668ab7eacf7bab3d9f38471380ab5f Mon Sep 17 00:00:00 2001
From: Enrico Flor <nericoflor@gmail.com>
Date: Fri, 16 Dec 2022 16:16:44 -0500
Subject: [PATCH] Add latex-table-wizard

---
 elpa-packages | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/elpa-packages b/elpa-packages
index 8254411cb2..b2ddceca10 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -78,7 +78,7 @@
   )

  (cdlatex		:url "https://github.com/cdominik/cdlatex")
-
+
  (cider			:url "https://github.com/clojure-emacs/cider"
   :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
   :news "CHANGELOG.md")
@@ -117,7 +117,7 @@
   :news "changelog.rst")

  (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
-
+
  (dracula-theme		:url "https://github.com/dracula/emacs"
   :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))

@@ -434,6 +434,11 @@
  (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
   :ignored-files ("doc" "test" "Cask" "Makefile"))

+ (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
+  :readme "description.txt"
+  :news "NEWS"
+  :doc "latex-table-wizard.texi")
+
  (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
   :readme "README.md")

--
2.38.1


[-- Attachment #1.3: Type: text/plain, Size: 11641 bytes --]



"Philip Kaludercic" <philipk@posteo.net> writes:

> Enrico Flor <enrico@eflor.net> writes:
>
>> I would like to submit latex-table-wizard to NonGNU ELPA.  This package
>> depends on AucTeX and on transient, and provides a minor mode with a
>> series of commands to navigate and edit complicated LaTeX table-like
>> environments (the standard ones, but the user can define additional
>> ones).
>>
>> With a transient UI, this package allows you to:
>>
>> + navigate "logically" (that is, move by cells)
>>
>> + insert or kill rows or column
>>
>> + move arbitrary cells or groups of cells around
>>
>> + align the table in different ways (however alignment is not needed for
>>   the functionalities above)
>>
>> These commands are not fooled by the presence of embedded tables or
>> other complications (for example: while editing a larger table, a buffer
>> substring like:
>>
>>     & ... \makecell{ a & b \\ c & d} ... &
>>
>> is still parsed as a single cell).
>>
>> From 27f25c72ed8e0e3e81cfc4f996f8c03c9c0155fe Mon Sep 17 00:00:00 2001
>> From: Enrico Flor <nericoflor@gmail.com>
>> Date: Fri, 16 Dec 2022 10:58:55 -0500
>> Subject: [PATCH] Add latex-table-wizard
>>
>> ---
>>  elpa-packages | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/elpa-packages b/elpa-packages
>> index 8254411cb2..90356989cb 100644
>> --- a/elpa-packages
>> +++ b/elpa-packages
>> @@ -78,7 +78,7 @@
>>    )
>>
>>   (cdlatex		:url "https://github.com/cdominik/cdlatex")
>> -
>> +
>>   (cider			:url "https://github.com/clojure-emacs/cider"
>>    :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
>>    :news "CHANGELOG.md")
>> @@ -117,7 +117,7 @@
>>    :news "changelog.rst")
>>
>>   (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
>> -
>> +
>>   (dracula-theme		:url "https://github.com/dracula/emacs"
>>    :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))
>>
>> @@ -434,6 +434,12 @@
>>   (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
>>    :ignored-files ("doc" "test" "Cask" "Makefile"))
>>
>> + (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
>
> Here are a few comments in diff-form, just from reading the code:
>
> diff --git a/latex-table-wizard.el b/latex-table-wizard.el
> index 0238ae3..0b487af 100644
> --- a/latex-table-wizard.el
> +++ b/latex-table-wizard.el
> @@ -90,7 +90,7 @@
>
>  (require 'latex)
>  (require 'seq)
> -(require 'rx)
> +(eval-when-compile (require 'rx))
>  (require 'regexp-opt)
>  (eval-when-compile (require 'subr-x))
>  (require 'transient)
> @@ -121,13 +121,11 @@ Capture group 1 matches the name of the macro.")
>
>  (defcustom latex-table-wizard-column-delimiters '("&")
>    "List of strings that are column delimiters if unescaped."
> -  :type '(repeat string)
> -  :group 'latex-table-wizard)
> +  :type '(repeat string))
>
>  (defcustom latex-table-wizard-row-delimiters '("\\\\\\\\")
>    "List of strings that are row delimiters if unescaped."
> -  :type '(repeat string)
> -  :group 'latex-table-wizard)
> +  :type '(repeat string))
>
>  (defcustom latex-table-wizard-hline-macros '("cline"
>                                               "vline"
> @@ -139,8 +137,7 @@ Capture group 1 matches the name of the macro.")
>
>  Each member of this list is a string that would be between the
>  \"\\\" and the arguments."
> -  :type '(repeat string)
> -  :group 'latex-table-wizard)
> +  :type '(repeat string))
>
>  (defcustom latex-table-wizard-new-environments-alist nil
>    "Alist mapping environment names to property lists.
> @@ -167,10 +164,9 @@ of a macro that inserts some horizontal line.  For a macro
>    :type '(alist :key-type (string :tag "Name of the environment:")
>                  :value-type (plist :key-type symbol
>                                     :options (:col :row :lines)
> -                                   :value-type (repeat string)))
> -
> -  :group 'latex-table-wizard)
> +                                   :value-type (repeat string))))
>
> +;; Why not use `memq'?
>  (defmacro latex-table-wizard--or (symbol &rest values)
>    "Return non-nil if SYMBOL is `eq' to one of VALUES."
>    (let ((bools (mapcar (lambda (value) `(eq ,symbol ,value))
> @@ -452,18 +448,15 @@ is a cons cell of the form (P . V), where P is either
>  If prop-val2 is nil, it is assumed that TABLE is a list of cells
>  that only differ for the property in the car of PROP-VAL1 (in
>  other words, that TABLE is either a column or a row)"
> -  (if prop-val2
> -      (catch 'cell
> +  (catch 'cell
> +    (if prop-val2
>          (dolist (x table)
>            (when (and (= (cdr prop-val1) (plist-get x (car prop-val1)))
>                       (= (cdr prop-val2) (plist-get x (car prop-val2))))
>              (throw 'cell x)))
> -        nil)
> -    (catch 'cell
>        (dolist (x table)
>          (when (= (cdr prop-val1) (plist-get x (car prop-val1)))
> -          (throw 'cell x)))
> -      nil)))
> +          (throw 'cell x))))))
>
>  (defun latex-table-wizard--sort (table same-line dir)
>    "Return a sorted table, column or row given TABLE.
> @@ -492,10 +485,10 @@ F, C precedes D and so on; and if DIR is either \\='next\\=' or
>           (copy-table (copy-sequence table)))
>      (if (not same-line)
>          (sort copy-table (lambda (x y)
> -                           (let ((rows `(,(plist-get x :row)
> -                                         ,(plist-get y :row)))
> -                                 (cols `(,(plist-get x :column)
> -                                         ,(plist-get y :column))))
> +                           (let ((rows (list (plist-get x :row)
> +                                             (plist-get y :row)))
> +                                 (cols (list (plist-get x :column)
> +                                             (plist-get y :column))))
>                               (cond ((and vert (apply #'= cols))
>                                      (apply #'< rows))
>                                     (vert
> @@ -536,10 +529,9 @@ beginning of the available portion of the buffer."
>          (catch 'found
>            (while (re-search-forward regexp nil t)
>              (when (<= (match-beginning group) position (match-end group))
> -              (throw 'found `(,(match-string-no-properties group)
> -                              ,(match-beginning group)
> -                              ,(match-end group))))
> -            nil))))))
> +              (throw 'found (list (match-string-no-properties group)
> +                                  (match-beginning group)
> +                                  (match-end group))))))))))
>
>  \f
>
> @@ -732,6 +724,9 @@ If SAME-LINE is non-nil, never leave current column or row."
>  X and Y are each a list of the form \\='(B E)\\=', where B and E
>  are markers corresponding to the beginning and the end of the
>  buffer substring."
> +  ;; Instead of assuming the form of X and Y, wouldn't it be better to
> +  ;; destruct these and make sure?  Then you could also avoid using
> +  ;; `apply'?
>    (save-excursion
>      (let ((x-string (concat " "
>                              (string-trim
> @@ -821,7 +816,7 @@ Don't print any message if NO-MESSAGE is non-nil."
>               (message "Cell (%s,%s) selected for swapping"
>                        (plist-get sel :column)
>                        (plist-get sel :row)))
> -           (latex-table-wizard--hl-cells `(,sel)))
> +           (latex-table-wizard--hl-cells (list sel)))
>            ((eq thing 'row)
>             (unless no-message
>               (message "Row %s selected for swapping"
> @@ -1261,44 +1256,10 @@ information about how transient suffixes are defined (that is,
>  how the data stored in this variable and in
>  `latex-table-wizard-default-keys' contributes to the definition
>  of the transient prefix)."
> -  :type '(alist :key-type
> -                (symbol :tag "Command:"
> -                        :options (toggle-truncate-lines
> -                                  exchange-point-and-mark
> -                                  universal-argument
> -                                  transient-quit-all
> -                                  undo
> -                                  latex-table-wizard-right
> -                                  latex-table-wizard-right
> -                                  latex-table-wizard-left
> -                                  latex-table-wizard-up
> -                                  latex-table-wizard-down
> -                                  latex-table-wizard-end-of-row
> -                                  latex-table-wizard-beginning-of-row
> -                                  latex-table-wizard-top
> -                                  latex-table-wizard-bottom
> -                                  latex-table-wizard-beginning-of-cell
> -                                  latex-table-wizard-end-of-cell
> -                                  latex-table-wizard-mark-cell
> -                                  latex-table-wizard-swap-cell-right
> -                                  latex-table-wizard-swap-cell-left
> -                                  latex-table-wizard-swap-cell-up
> -                                  latex-table-wizard-swap-cell-down
> -                                  latex-table-wizard-swap-column-right
> -                                  latex-table-wizard-swap-column-left
> -                                  latex-table-wizard-swap-row-up
> -                                  latex-table-wizard-swap-row-down
> -                                  latex-table-wizard-align
> -                                  latex-table-wizard-select-column
> -                                  latex-table-wizard-select-row
> -                                  latex-table-wizard-select-deselect-cell
> -                                  latex-table-wizard-deselect-all
> -                                  latex-table-wizard-swap
> -                                  latex-table-wizard-insert-column
> -                                  latex-table-wizard-insert-row
> -                                  latex-table-wizard-kill-column
> -                                  latex-table-wizard-kill-row))
> -                :value-type string)
> +  :type `(alist :key-type
> +                (symbol :tag "Command:")
> +                :value-type string
> +                :options ,(mapcar #'car latex-table-wizard-default-keys))
>    :group 'latex-table-wizard)
>
>  (defun latex-table-wizard--make-suffix (symbol)
>
>> +  :readme "readme.org"
>
> Are you sure you want to use your "readme.org" file as the package
> "readme"?  The contents will be displayed when a user uses
> describe-package to find out more about what latex-table-wizard does,
> and in my experience it is better to keep it short instead of presenting
> a wall of text, that is usually better kept part of the manual.
>
>> +  :doc "latex-table-wizard.texi"
>
> It also appears you have a .info file in your repository that isn't
> needed.  That will be generated by the ELPA server, so you don't need to
> track the "binary" file in your repository.
>
> If you _do_ intent to build the manual yourself, you should mention the
> .info file here, not the .texi file.
>
>> +  :news "NEWS"
>> +  :ignored-files ("COPYING"))
>
> Instead of listing files in the package specification, it would be
> better to maintain a .elpaignore file in your own repository.
>
>>   (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
>>    :readme "README.md")
>>
>> --
>> 2.38.1

[-- Attachment #1.4: publickey - enrico@eflor.net - ffbd1445.asc --]
[-- Type: application/pgp-keys, Size: 697 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 21:29   ` Enrico Flor
@ 2022-12-16 21:37     ` Philip Kaludercic
  2022-12-16 21:59       ` Enrico Flor
  2022-12-17  6:22       ` tomas
  0 siblings, 2 replies; 11+ messages in thread
From: Philip Kaludercic @ 2022-12-16 21:37 UTC (permalink / raw)
  To: Enrico Flor; +Cc: emacs-devel

Enrico Flor <enrico@eflor.net> writes:

> Thank you so much for your comments!  I implemented your many
> suggestions wrt. the code.  I must say I didn't use to have all the
> backquotes but then I read somewhere that you should prefer
>
>   `(,x ,y)
>
> over
>
>   (list x y)
>
> and so I replaced all the instances of the latter with the former.  I
> probably misunderstood the "advice".

I cannot think of why, after all

  (macroexpand-all '`(,a ,b ,c)) => (list a b c)

and I prefer the latter, as it just seems cleaner.  I'd be curious to
hear the original argument, because to me that sounds like using a
feature for it's own sake.

> I also added the .elpaignore, removed the .info file and added a short
> description.txt file to serve as readme. 

That might work as well, but I think you might also use the ";;;
Commentary" section in the main file for the same purpose.

> From c711b1b314668ab7eacf7bab3d9f38471380ab5f Mon Sep 17 00:00:00 2001
> From: Enrico Flor <nericoflor@gmail.com>
> Date: Fri, 16 Dec 2022 16:16:44 -0500
> Subject: [PATCH] Add latex-table-wizard
>
> ---
>  elpa-packages | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/elpa-packages b/elpa-packages
> index 8254411cb2..b2ddceca10 100644
> --- a/elpa-packages
> +++ b/elpa-packages
> @@ -78,7 +78,7 @@
>    )
>
>   (cdlatex		:url "https://github.com/cdominik/cdlatex")
> -
> +
>   (cider			:url "https://github.com/clojure-emacs/cider"
>    :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
>    :news "CHANGELOG.md")
> @@ -117,7 +117,7 @@
>    :news "changelog.rst")
>
>   (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
> -
> +
>   (dracula-theme		:url "https://github.com/dracula/emacs"
>    :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))
>
> @@ -434,6 +434,11 @@
>   (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
>    :ignored-files ("doc" "test" "Cask" "Makefile"))
>
> + (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
> +  :readme "description.txt"
> +  :news "NEWS"
> +  :doc "latex-table-wizard.texi")
> +
>   (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
>    :readme "README.md")
>
> --
> 2.38.1
>
>
>
> "Philip Kaludercic" <philipk@posteo.net> writes:
>
>> Enrico Flor <enrico@eflor.net> writes:
>>
>>> I would like to submit latex-table-wizard to NonGNU ELPA.  This package
>>> depends on AucTeX and on transient, and provides a minor mode with a
>>> series of commands to navigate and edit complicated LaTeX table-like
>>> environments (the standard ones, but the user can define additional
>>> ones).
>>>
>>> With a transient UI, this package allows you to:
>>>
>>> + navigate "logically" (that is, move by cells)
>>>
>>> + insert or kill rows or column
>>>
>>> + move arbitrary cells or groups of cells around
>>>
>>> + align the table in different ways (however alignment is not needed for
>>>   the functionalities above)
>>>
>>> These commands are not fooled by the presence of embedded tables or
>>> other complications (for example: while editing a larger table, a buffer
>>> substring like:
>>>
>>>     & ... \makecell{ a & b \\ c & d} ... &
>>>
>>> is still parsed as a single cell).
>>>
>>> From 27f25c72ed8e0e3e81cfc4f996f8c03c9c0155fe Mon Sep 17 00:00:00 2001
>>> From: Enrico Flor <nericoflor@gmail.com>
>>> Date: Fri, 16 Dec 2022 10:58:55 -0500
>>> Subject: [PATCH] Add latex-table-wizard
>>>
>>> ---
>>>  elpa-packages | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/elpa-packages b/elpa-packages
>>> index 8254411cb2..90356989cb 100644
>>> --- a/elpa-packages
>>> +++ b/elpa-packages
>>> @@ -78,7 +78,7 @@
>>>    )
>>>
>>>   (cdlatex		:url "https://github.com/cdominik/cdlatex")
>>> -
>>> +
>>>   (cider			:url "https://github.com/clojure-emacs/cider"
>>>    :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
>>>    :news "CHANGELOG.md")
>>> @@ -117,7 +117,7 @@
>>>    :news "changelog.rst")
>>>
>>>   (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
>>> -
>>> +
>>>   (dracula-theme		:url "https://github.com/dracula/emacs"
>>>    :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))
>>>
>>> @@ -434,6 +434,12 @@
>>>   (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
>>>    :ignored-files ("doc" "test" "Cask" "Makefile"))
>>>
>>> + (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
>>
>> Here are a few comments in diff-form, just from reading the code:
>>
>> diff --git a/latex-table-wizard.el b/latex-table-wizard.el
>> index 0238ae3..0b487af 100644
>> --- a/latex-table-wizard.el
>> +++ b/latex-table-wizard.el
>> @@ -90,7 +90,7 @@
>>
>>  (require 'latex)
>>  (require 'seq)
>> -(require 'rx)
>> +(eval-when-compile (require 'rx))
>>  (require 'regexp-opt)
>>  (eval-when-compile (require 'subr-x))
>>  (require 'transient)
>> @@ -121,13 +121,11 @@ Capture group 1 matches the name of the macro.")
>>
>>  (defcustom latex-table-wizard-column-delimiters '("&")
>>    "List of strings that are column delimiters if unescaped."
>> -  :type '(repeat string)
>> -  :group 'latex-table-wizard)
>> +  :type '(repeat string))
>>
>>  (defcustom latex-table-wizard-row-delimiters '("\\\\\\\\")
>>    "List of strings that are row delimiters if unescaped."
>> -  :type '(repeat string)
>> -  :group 'latex-table-wizard)
>> +  :type '(repeat string))
>>
>>  (defcustom latex-table-wizard-hline-macros '("cline"
>>                                               "vline"
>> @@ -139,8 +137,7 @@ Capture group 1 matches the name of the macro.")
>>
>>  Each member of this list is a string that would be between the
>>  \"\\\" and the arguments."
>> -  :type '(repeat string)
>> -  :group 'latex-table-wizard)
>> +  :type '(repeat string))
>>
>>  (defcustom latex-table-wizard-new-environments-alist nil
>>    "Alist mapping environment names to property lists.
>> @@ -167,10 +164,9 @@ of a macro that inserts some horizontal line.  For a macro
>>    :type '(alist :key-type (string :tag "Name of the environment:")
>>                  :value-type (plist :key-type symbol
>>                                     :options (:col :row :lines)
>> -                                   :value-type (repeat string)))
>> -
>> -  :group 'latex-table-wizard)
>> +                                   :value-type (repeat string))))
>>
>> +;; Why not use `memq'?
>>  (defmacro latex-table-wizard--or (symbol &rest values)
>>    "Return non-nil if SYMBOL is `eq' to one of VALUES."
>>    (let ((bools (mapcar (lambda (value) `(eq ,symbol ,value))
>> @@ -452,18 +448,15 @@ is a cons cell of the form (P . V), where P is either
>>  If prop-val2 is nil, it is assumed that TABLE is a list of cells
>>  that only differ for the property in the car of PROP-VAL1 (in
>>  other words, that TABLE is either a column or a row)"
>> -  (if prop-val2
>> -      (catch 'cell
>> +  (catch 'cell
>> +    (if prop-val2
>>          (dolist (x table)
>>            (when (and (= (cdr prop-val1) (plist-get x (car prop-val1)))
>>                       (= (cdr prop-val2) (plist-get x (car prop-val2))))
>>              (throw 'cell x)))
>> -        nil)
>> -    (catch 'cell
>>        (dolist (x table)
>>          (when (= (cdr prop-val1) (plist-get x (car prop-val1)))
>> -          (throw 'cell x)))
>> -      nil)))
>> +          (throw 'cell x))))))
>>
>>  (defun latex-table-wizard--sort (table same-line dir)
>>    "Return a sorted table, column or row given TABLE.
>> @@ -492,10 +485,10 @@ F, C precedes D and so on; and if DIR is either \\='next\\=' or
>>           (copy-table (copy-sequence table)))
>>      (if (not same-line)
>>          (sort copy-table (lambda (x y)
>> -                           (let ((rows `(,(plist-get x :row)
>> -                                         ,(plist-get y :row)))
>> -                                 (cols `(,(plist-get x :column)
>> -                                         ,(plist-get y :column))))
>> +                           (let ((rows (list (plist-get x :row)
>> +                                             (plist-get y :row)))
>> +                                 (cols (list (plist-get x :column)
>> +                                             (plist-get y :column))))
>>                               (cond ((and vert (apply #'= cols))
>>                                      (apply #'< rows))
>>                                     (vert
>> @@ -536,10 +529,9 @@ beginning of the available portion of the buffer."
>>          (catch 'found
>>            (while (re-search-forward regexp nil t)
>>              (when (<= (match-beginning group) position (match-end group))
>> -              (throw 'found `(,(match-string-no-properties group)
>> -                              ,(match-beginning group)
>> -                              ,(match-end group))))
>> -            nil))))))
>> +              (throw 'found (list (match-string-no-properties group)
>> +                                  (match-beginning group)
>> +                                  (match-end group))))))))))
>>
>>  \f
>>
>> @@ -732,6 +724,9 @@ If SAME-LINE is non-nil, never leave current column or row."
>>  X and Y are each a list of the form \\='(B E)\\=', where B and E
>>  are markers corresponding to the beginning and the end of the
>>  buffer substring."
>> +  ;; Instead of assuming the form of X and Y, wouldn't it be better to
>> +  ;; destruct these and make sure?  Then you could also avoid using
>> +  ;; `apply'?
>>    (save-excursion
>>      (let ((x-string (concat " "
>>                              (string-trim
>> @@ -821,7 +816,7 @@ Don't print any message if NO-MESSAGE is non-nil."
>>               (message "Cell (%s,%s) selected for swapping"
>>                        (plist-get sel :column)
>>                        (plist-get sel :row)))
>> -           (latex-table-wizard--hl-cells `(,sel)))
>> +           (latex-table-wizard--hl-cells (list sel)))
>>            ((eq thing 'row)
>>             (unless no-message
>>               (message "Row %s selected for swapping"
>> @@ -1261,44 +1256,10 @@ information about how transient suffixes are defined (that is,
>>  how the data stored in this variable and in
>>  `latex-table-wizard-default-keys' contributes to the definition
>>  of the transient prefix)."
>> -  :type '(alist :key-type
>> -                (symbol :tag "Command:"
>> -                        :options (toggle-truncate-lines
>> -                                  exchange-point-and-mark
>> -                                  universal-argument
>> -                                  transient-quit-all
>> -                                  undo
>> -                                  latex-table-wizard-right
>> -                                  latex-table-wizard-right
>> -                                  latex-table-wizard-left
>> -                                  latex-table-wizard-up
>> -                                  latex-table-wizard-down
>> -                                  latex-table-wizard-end-of-row
>> -                                  latex-table-wizard-beginning-of-row
>> -                                  latex-table-wizard-top
>> -                                  latex-table-wizard-bottom
>> -                                  latex-table-wizard-beginning-of-cell
>> -                                  latex-table-wizard-end-of-cell
>> -                                  latex-table-wizard-mark-cell
>> -                                  latex-table-wizard-swap-cell-right
>> -                                  latex-table-wizard-swap-cell-left
>> -                                  latex-table-wizard-swap-cell-up
>> -                                  latex-table-wizard-swap-cell-down
>> -                                  latex-table-wizard-swap-column-right
>> -                                  latex-table-wizard-swap-column-left
>> -                                  latex-table-wizard-swap-row-up
>> -                                  latex-table-wizard-swap-row-down
>> -                                  latex-table-wizard-align
>> -                                  latex-table-wizard-select-column
>> -                                  latex-table-wizard-select-row
>> -                                  latex-table-wizard-select-deselect-cell
>> -                                  latex-table-wizard-deselect-all
>> -                                  latex-table-wizard-swap
>> -                                  latex-table-wizard-insert-column
>> -                                  latex-table-wizard-insert-row
>> -                                  latex-table-wizard-kill-column
>> -                                  latex-table-wizard-kill-row))
>> -                :value-type string)
>> +  :type `(alist :key-type
>> +                (symbol :tag "Command:")
>> +                :value-type string
>> +                :options ,(mapcar #'car latex-table-wizard-default-keys))
>>    :group 'latex-table-wizard)
>>
>>  (defun latex-table-wizard--make-suffix (symbol)
>>
>>> +  :readme "readme.org"
>>
>> Are you sure you want to use your "readme.org" file as the package
>> "readme"?  The contents will be displayed when a user uses
>> describe-package to find out more about what latex-table-wizard does,
>> and in my experience it is better to keep it short instead of presenting
>> a wall of text, that is usually better kept part of the manual.
>>
>>> +  :doc "latex-table-wizard.texi"
>>
>> It also appears you have a .info file in your repository that isn't
>> needed.  That will be generated by the ELPA server, so you don't need to
>> track the "binary" file in your repository.
>>
>> If you _do_ intent to build the manual yourself, you should mention the
>> .info file here, not the .texi file.
>>
>>> +  :news "NEWS"
>>> +  :ignored-files ("COPYING"))
>>
>> Instead of listing files in the package specification, it would be
>> better to maintain a .elpaignore file in your own repository.
>>
>>>   (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
>>>    :readme "README.md")
>>>
>>> --
>>> 2.38.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 21:37     ` Philip Kaludercic
@ 2022-12-16 21:59       ` Enrico Flor
  2022-12-16 22:58         ` Philip Kaludercic
  2022-12-17  6:22       ` tomas
  1 sibling, 1 reply; 11+ messages in thread
From: Enrico Flor @ 2022-12-16 21:59 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 14999 bytes --]

"Philip Kaludercic" <philipk@posteo.net> writes:

> Enrico Flor <enrico@eflor.net> writes:
>
>> Thank you so much for your comments!  I implemented your many
>> suggestions wrt. the code.  I must say I didn't use to have all the
>> backquotes but then I read somewhere that you should prefer
>>
>>   `(,x ,y)
>>
>> over
>>
>>   (list x y)
>>
>> and so I replaced all the instances of the latter with the former.  I
>> probably misunderstood the "advice".
>
> I cannot think of why, after all
>
>   (macroexpand-all '`(,a ,b ,c)) => (list a b c)
>
> and I prefer the latter, as it just seems cleaner.  I'd be curious to
> hear the original argument, because to me that sounds like using a
> feature for it's own sake.
>

I agree it seems cleaner.  I went back to check for that claim, and
indeed... I did misunderstand.  It's in the "The Emacs Lisp Style Guide"
on github, but what it says is that syntax quoting should be preferred
*in macro definitions*.

>> I also added the .elpaignore, removed the .info file and added a short
>> description.txt file to serve as readme.
>
> That might work as well, but I think you might also use the ";;;
> Commentary" section in the main file for the same purpose.

I think I'd rather have something shorter than that, and keep more stuff
in the Commentary section.  But ultimately, I'll do what people tell me
to do here.

>> From c711b1b314668ab7eacf7bab3d9f38471380ab5f Mon Sep 17 00:00:00 2001
>> From: Enrico Flor <nericoflor@gmail.com>
>> Date: Fri, 16 Dec 2022 16:16:44 -0500
>> Subject: [PATCH] Add latex-table-wizard
>>
>> ---
>>  elpa-packages | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/elpa-packages b/elpa-packages
>> index 8254411cb2..b2ddceca10 100644
>> --- a/elpa-packages
>> +++ b/elpa-packages
>> @@ -78,7 +78,7 @@
>>    )
>>
>>   (cdlatex		:url "https://github.com/cdominik/cdlatex")
>> -
>> +
>>   (cider			:url "https://github.com/clojure-emacs/cider"
>>    :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
>>    :news "CHANGELOG.md")
>> @@ -117,7 +117,7 @@
>>    :news "changelog.rst")
>>
>>   (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
>> -
>> +
>>   (dracula-theme		:url "https://github.com/dracula/emacs"
>>    :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))
>>
>> @@ -434,6 +434,11 @@
>>   (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
>>    :ignored-files ("doc" "test" "Cask" "Makefile"))
>>
>> + (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
>> +  :readme "description.txt"
>> +  :news "NEWS"
>> +  :doc "latex-table-wizard.texi")
>> +
>>   (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
>>    :readme "README.md")
>>
>> --
>> 2.38.1
>>
>>
>>
>> "Philip Kaludercic" <philipk@posteo.net> writes:
>>
>>> Enrico Flor <enrico@eflor.net> writes:
>>>
>>>> I would like to submit latex-table-wizard to NonGNU ELPA.  This package
>>>> depends on AucTeX and on transient, and provides a minor mode with a
>>>> series of commands to navigate and edit complicated LaTeX table-like
>>>> environments (the standard ones, but the user can define additional
>>>> ones).
>>>>
>>>> With a transient UI, this package allows you to:
>>>>
>>>> + navigate "logically" (that is, move by cells)
>>>>
>>>> + insert or kill rows or column
>>>>
>>>> + move arbitrary cells or groups of cells around
>>>>
>>>> + align the table in different ways (however alignment is not needed for
>>>>   the functionalities above)
>>>>
>>>> These commands are not fooled by the presence of embedded tables or
>>>> other complications (for example: while editing a larger table, a buffer
>>>> substring like:
>>>>
>>>>     & ... \makecell{ a & b \\ c & d} ... &
>>>>
>>>> is still parsed as a single cell).
>>>>
>>>> From 27f25c72ed8e0e3e81cfc4f996f8c03c9c0155fe Mon Sep 17 00:00:00 2001
>>>> From: Enrico Flor <nericoflor@gmail.com>
>>>> Date: Fri, 16 Dec 2022 10:58:55 -0500
>>>> Subject: [PATCH] Add latex-table-wizard
>>>>
>>>> ---
>>>>  elpa-packages | 10 ++++++++--
>>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/elpa-packages b/elpa-packages
>>>> index 8254411cb2..90356989cb 100644
>>>> --- a/elpa-packages
>>>> +++ b/elpa-packages
>>>> @@ -78,7 +78,7 @@
>>>>    )
>>>>
>>>>   (cdlatex		:url "https://github.com/cdominik/cdlatex")
>>>> -
>>>> +
>>>>   (cider			:url "https://github.com/clojure-emacs/cider"
>>>>    :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
>>>>    :news "CHANGELOG.md")
>>>> @@ -117,7 +117,7 @@
>>>>    :news "changelog.rst")
>>>>
>>>>   (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
>>>> -
>>>> +
>>>>   (dracula-theme		:url "https://github.com/dracula/emacs"
>>>>    :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))
>>>>
>>>> @@ -434,6 +434,12 @@
>>>>   (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
>>>>    :ignored-files ("doc" "test" "Cask" "Makefile"))
>>>>
>>>> + (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
>>>
>>> Here are a few comments in diff-form, just from reading the code:
>>>
>>> diff --git a/latex-table-wizard.el b/latex-table-wizard.el
>>> index 0238ae3..0b487af 100644
>>> --- a/latex-table-wizard.el
>>> +++ b/latex-table-wizard.el
>>> @@ -90,7 +90,7 @@
>>>
>>>  (require 'latex)
>>>  (require 'seq)
>>> -(require 'rx)
>>> +(eval-when-compile (require 'rx))
>>>  (require 'regexp-opt)
>>>  (eval-when-compile (require 'subr-x))
>>>  (require 'transient)
>>> @@ -121,13 +121,11 @@ Capture group 1 matches the name of the macro.")
>>>
>>>  (defcustom latex-table-wizard-column-delimiters '("&")
>>>    "List of strings that are column delimiters if unescaped."
>>> -  :type '(repeat string)
>>> -  :group 'latex-table-wizard)
>>> +  :type '(repeat string))
>>>
>>>  (defcustom latex-table-wizard-row-delimiters '("\\\\\\\\")
>>>    "List of strings that are row delimiters if unescaped."
>>> -  :type '(repeat string)
>>> -  :group 'latex-table-wizard)
>>> +  :type '(repeat string))
>>>
>>>  (defcustom latex-table-wizard-hline-macros '("cline"
>>>                                               "vline"
>>> @@ -139,8 +137,7 @@ Capture group 1 matches the name of the macro.")
>>>
>>>  Each member of this list is a string that would be between the
>>>  \"\\\" and the arguments."
>>> -  :type '(repeat string)
>>> -  :group 'latex-table-wizard)
>>> +  :type '(repeat string))
>>>
>>>  (defcustom latex-table-wizard-new-environments-alist nil
>>>    "Alist mapping environment names to property lists.
>>> @@ -167,10 +164,9 @@ of a macro that inserts some horizontal line.  For a macro
>>>    :type '(alist :key-type (string :tag "Name of the environment:")
>>>                  :value-type (plist :key-type symbol
>>>                                     :options (:col :row :lines)
>>> -                                   :value-type (repeat string)))
>>> -
>>> -  :group 'latex-table-wizard)
>>> +                                   :value-type (repeat string))))
>>>
>>> +;; Why not use `memq'?
>>>  (defmacro latex-table-wizard--or (symbol &rest values)
>>>    "Return non-nil if SYMBOL is `eq' to one of VALUES."
>>>    (let ((bools (mapcar (lambda (value) `(eq ,symbol ,value))
>>> @@ -452,18 +448,15 @@ is a cons cell of the form (P . V), where P is either
>>>  If prop-val2 is nil, it is assumed that TABLE is a list of cells
>>>  that only differ for the property in the car of PROP-VAL1 (in
>>>  other words, that TABLE is either a column or a row)"
>>> -  (if prop-val2
>>> -      (catch 'cell
>>> +  (catch 'cell
>>> +    (if prop-val2
>>>          (dolist (x table)
>>>            (when (and (= (cdr prop-val1) (plist-get x (car prop-val1)))
>>>                       (= (cdr prop-val2) (plist-get x (car prop-val2))))
>>>              (throw 'cell x)))
>>> -        nil)
>>> -    (catch 'cell
>>>        (dolist (x table)
>>>          (when (= (cdr prop-val1) (plist-get x (car prop-val1)))
>>> -          (throw 'cell x)))
>>> -      nil)))
>>> +          (throw 'cell x))))))
>>>
>>>  (defun latex-table-wizard--sort (table same-line dir)
>>>    "Return a sorted table, column or row given TABLE.
>>> @@ -492,10 +485,10 @@ F, C precedes D and so on; and if DIR is either \\='next\\=' or
>>>           (copy-table (copy-sequence table)))
>>>      (if (not same-line)
>>>          (sort copy-table (lambda (x y)
>>> -                           (let ((rows `(,(plist-get x :row)
>>> -                                         ,(plist-get y :row)))
>>> -                                 (cols `(,(plist-get x :column)
>>> -                                         ,(plist-get y :column))))
>>> +                           (let ((rows (list (plist-get x :row)
>>> +                                             (plist-get y :row)))
>>> +                                 (cols (list (plist-get x :column)
>>> +                                             (plist-get y :column))))
>>>                               (cond ((and vert (apply #'= cols))
>>>                                      (apply #'< rows))
>>>                                     (vert
>>> @@ -536,10 +529,9 @@ beginning of the available portion of the buffer."
>>>          (catch 'found
>>>            (while (re-search-forward regexp nil t)
>>>              (when (<= (match-beginning group) position (match-end group))
>>> -              (throw 'found `(,(match-string-no-properties group)
>>> -                              ,(match-beginning group)
>>> -                              ,(match-end group))))
>>> -            nil))))))
>>> +              (throw 'found (list (match-string-no-properties group)
>>> +                                  (match-beginning group)
>>> +                                  (match-end group))))))))))
>>>
>>>  \f
>>>
>>> @@ -732,6 +724,9 @@ If SAME-LINE is non-nil, never leave current column or row."
>>>  X and Y are each a list of the form \\='(B E)\\=', where B and E
>>>  are markers corresponding to the beginning and the end of the
>>>  buffer substring."
>>> +  ;; Instead of assuming the form of X and Y, wouldn't it be better to
>>> +  ;; destruct these and make sure?  Then you could also avoid using
>>> +  ;; `apply'?
>>>    (save-excursion
>>>      (let ((x-string (concat " "
>>>                              (string-trim
>>> @@ -821,7 +816,7 @@ Don't print any message if NO-MESSAGE is non-nil."
>>>               (message "Cell (%s,%s) selected for swapping"
>>>                        (plist-get sel :column)
>>>                        (plist-get sel :row)))
>>> -           (latex-table-wizard--hl-cells `(,sel)))
>>> +           (latex-table-wizard--hl-cells (list sel)))
>>>            ((eq thing 'row)
>>>             (unless no-message
>>>               (message "Row %s selected for swapping"
>>> @@ -1261,44 +1256,10 @@ information about how transient suffixes are defined (that is,
>>>  how the data stored in this variable and in
>>>  `latex-table-wizard-default-keys' contributes to the definition
>>>  of the transient prefix)."
>>> -  :type '(alist :key-type
>>> -                (symbol :tag "Command:"
>>> -                        :options (toggle-truncate-lines
>>> -                                  exchange-point-and-mark
>>> -                                  universal-argument
>>> -                                  transient-quit-all
>>> -                                  undo
>>> -                                  latex-table-wizard-right
>>> -                                  latex-table-wizard-right
>>> -                                  latex-table-wizard-left
>>> -                                  latex-table-wizard-up
>>> -                                  latex-table-wizard-down
>>> -                                  latex-table-wizard-end-of-row
>>> -                                  latex-table-wizard-beginning-of-row
>>> -                                  latex-table-wizard-top
>>> -                                  latex-table-wizard-bottom
>>> -                                  latex-table-wizard-beginning-of-cell
>>> -                                  latex-table-wizard-end-of-cell
>>> -                                  latex-table-wizard-mark-cell
>>> -                                  latex-table-wizard-swap-cell-right
>>> -                                  latex-table-wizard-swap-cell-left
>>> -                                  latex-table-wizard-swap-cell-up
>>> -                                  latex-table-wizard-swap-cell-down
>>> -                                  latex-table-wizard-swap-column-right
>>> -                                  latex-table-wizard-swap-column-left
>>> -                                  latex-table-wizard-swap-row-up
>>> -                                  latex-table-wizard-swap-row-down
>>> -                                  latex-table-wizard-align
>>> -                                  latex-table-wizard-select-column
>>> -                                  latex-table-wizard-select-row
>>> -                                  latex-table-wizard-select-deselect-cell
>>> -                                  latex-table-wizard-deselect-all
>>> -                                  latex-table-wizard-swap
>>> -                                  latex-table-wizard-insert-column
>>> -                                  latex-table-wizard-insert-row
>>> -                                  latex-table-wizard-kill-column
>>> -                                  latex-table-wizard-kill-row))
>>> -                :value-type string)
>>> +  :type `(alist :key-type
>>> +                (symbol :tag "Command:")
>>> +                :value-type string
>>> +                :options ,(mapcar #'car latex-table-wizard-default-keys))
>>>    :group 'latex-table-wizard)
>>>
>>>  (defun latex-table-wizard--make-suffix (symbol)
>>>
>>>> +  :readme "readme.org"
>>>
>>> Are you sure you want to use your "readme.org" file as the package
>>> "readme"?  The contents will be displayed when a user uses
>>> describe-package to find out more about what latex-table-wizard does,
>>> and in my experience it is better to keep it short instead of presenting
>>> a wall of text, that is usually better kept part of the manual.
>>>
>>>> +  :doc "latex-table-wizard.texi"
>>>
>>> It also appears you have a .info file in your repository that isn't
>>> needed.  That will be generated by the ELPA server, so you don't need to
>>> track the "binary" file in your repository.
>>>
>>> If you _do_ intent to build the manual yourself, you should mention the
>>> .info file here, not the .texi file.
>>>
>>>> +  :news "NEWS"
>>>> +  :ignored-files ("COPYING"))
>>>
>>> Instead of listing files in the package specification, it would be
>>> better to maintain a .elpaignore file in your own repository.
>>>
>>>>   (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
>>>>    :readme "README.md")
>>>>
>>>> --
>>>> 2.38.1

[-- Attachment #1.1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

[-- Attachment #1.2: publickey - enrico@eflor.net - ffbd1445.asc --]
[-- Type: application/pgp-keys, Size: 697 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 21:59       ` Enrico Flor
@ 2022-12-16 22:58         ` Philip Kaludercic
  2022-12-16 23:36           ` Enrico Flor
  0 siblings, 1 reply; 11+ messages in thread
From: Philip Kaludercic @ 2022-12-16 22:58 UTC (permalink / raw)
  To: Enrico Flor; +Cc: emacs-devel

Enrico Flor <enrico@eflor.net> writes:

> "Philip Kaludercic" <philipk@posteo.net> writes:
>
>> Enrico Flor <enrico@eflor.net> writes:
>>
>>> Thank you so much for your comments!  I implemented your many
>>> suggestions wrt. the code.  I must say I didn't use to have all the
>>> backquotes but then I read somewhere that you should prefer
>>>
>>>   `(,x ,y)
>>>
>>> over
>>>
>>>   (list x y)
>>>
>>> and so I replaced all the instances of the latter with the former.  I
>>> probably misunderstood the "advice".
>>
>> I cannot think of why, after all
>>
>>   (macroexpand-all '`(,a ,b ,c)) => (list a b c)
>>
>> and I prefer the latter, as it just seems cleaner.  I'd be curious to
>> hear the original argument, because to me that sounds like using a
>> feature for it's own sake.
>>
>
> I agree it seems cleaner.  I went back to check for that claim, and
> indeed... I did misunderstand.  It's in the "The Emacs Lisp Style Guide"
> on github, but what it says is that syntax quoting should be preferred
> *in macro definitions*.

Ah, of course because using backquoting allows you to write data that
looks like code.

>>> I also added the .elpaignore, removed the .info file and added a short
>>> description.txt file to serve as readme.
>>
>> That might work as well, but I think you might also use the ";;;
>> Commentary" section in the main file for the same purpose.
>
> I think I'd rather have something shorter than that, and keep more stuff
> in the Commentary section.  But ultimately, I'll do what people tell me
> to do here.

All I have been providing are a few optional suggestions and giving you
information you might or might not know, you are the best judge of what
you want the package to look like.  If you say you want a
"description.txt" file to contain a description of the package, that is
absolutely fine.  Another option is just a plain "README" file, without
any extension.  ELPA detects these by default, so the package
specification wouldn't have to list it.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 22:58         ` Philip Kaludercic
@ 2022-12-16 23:36           ` Enrico Flor
  0 siblings, 0 replies; 11+ messages in thread
From: Enrico Flor @ 2022-12-16 23:36 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 754 bytes --]

"Philip Kaludercic" <philipk@posteo.net> writes:

>
> All I have been providing are a few optional suggestions and giving you
> information you might or might not know, you are the best judge of what
> you want the package to look like.  If you say you want a
> "description.txt" file to contain a description of the package, that is
> absolutely fine.  Another option is just a plain "README" file, without
> any extension.  ELPA detects these by default, so the package
> specification wouldn't have to list it.

And I thank you for that!  At the end I think if ELPA extracts the
readme for C-h P etc. from the Commentary section, that's all that is
needed.  So I removed the :readme value altogether and leave it at the
Commentary, as you suggested.


[-- Attachment #1.1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

[-- Attachment #1.2: patch --]
[-- Type: text/x-patch, Size: 1295 bytes --]

From c58ee759cf92c4dca306f66d3415eb34732224cb Mon Sep 17 00:00:00 2001
From: Enrico Flor <nericoflor@gmail.com>
Date: Fri, 16 Dec 2022 18:28:26 -0500
Subject: [PATCH] latex-table-wizard added

---
 elpa-packages | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/elpa-packages b/elpa-packages
index 8254411cb2..3e3579516e 100644
--- a/elpa-packages
+++ b/elpa-packages
@@ -78,7 +78,7 @@
   )

  (cdlatex		:url "https://github.com/cdominik/cdlatex")
-
+
  (cider			:url "https://github.com/clojure-emacs/cider"
   :ignored-files ("LICENSE" "doc" "logo" "refcard" "test")
   :news "CHANGELOG.md")
@@ -117,7 +117,7 @@
   :news "changelog.rst")

  (dockerfile-mode	:url "https://github.com/spotify/dockerfile-mode")
-
+
  (dracula-theme		:url "https://github.com/dracula/emacs"
   :ignored-files ("INSTALL.md" "screenshot.png" "start_emacs_test.sh" "test-profile.el"))

@@ -434,6 +434,10 @@
  (kotlin-mode		:url "https://github.com/Emacs-Kotlin-Mode-Maintainers/kotlin-mode"
   :ignored-files ("doc" "test" "Cask" "Makefile"))

+ (latex-table-wizard    :url "https://github.com/enricoflor/latex-table-wizard"
+  :news "NEWS"
+  :doc "latex-table-wizard.texi")
+
  (lorem-ipsum           :url "https://github.com/jschaf/emacs-lorem-ipsum"
   :readme "README.md")

--
2.38.1


[-- Attachment #1.3: publickey - enrico@eflor.net - ffbd1445.asc --]
[-- Type: application/pgp-keys, Size: 697 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 21:37     ` Philip Kaludercic
  2022-12-16 21:59       ` Enrico Flor
@ 2022-12-17  6:22       ` tomas
  2022-12-17 10:01         ` Philip Kaludercic
  2022-12-17 16:13         ` [External] : " Drew Adams
  1 sibling, 2 replies; 11+ messages in thread
From: tomas @ 2022-12-17  6:22 UTC (permalink / raw)
  To: emacs-devel

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

On Fri, Dec 16, 2022 at 09:37:23PM +0000, Philip Kaludercic wrote:
> Enrico Flor <enrico@eflor.net> writes:
> 
> > Thank you so much for your comments!  I implemented your many
> > suggestions wrt. the code.  I must say I didn't use to have all the
> > backquotes but then I read somewhere that you should prefer
> >
> >   `(,x ,y)
> >
> > over
> >
> >   (list x y)

[...]

> I cannot think of why, after all
> 
>   (macroexpand-all '`(,a ,b ,c)) => (list a b c)

I think it's a question of style. As you, Philip, note downthread, I
usually decide on whether I see [1] the context as "data with some
interspersed (live) values" (think "json with variable interpolation"
or as "program". Lisp itself is ambiguous about that :-)

Cheers

[1] and want to convey to my readers

-- 
t

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-17  6:22       ` tomas
@ 2022-12-17 10:01         ` Philip Kaludercic
  2022-12-17 16:13         ` [External] : " Drew Adams
  1 sibling, 0 replies; 11+ messages in thread
From: Philip Kaludercic @ 2022-12-17 10:01 UTC (permalink / raw)
  To: tomas; +Cc: emacs-devel

<tomas@tuxteam.de> writes:

> On Fri, Dec 16, 2022 at 09:37:23PM +0000, Philip Kaludercic wrote:
>> Enrico Flor <enrico@eflor.net> writes:
>> 
>> > Thank you so much for your comments!  I implemented your many
>> > suggestions wrt. the code.  I must say I didn't use to have all the
>> > backquotes but then I read somewhere that you should prefer
>> >
>> >   `(,x ,y)
>> >
>> > over
>> >
>> >   (list x y)
>
> [...]
>
>> I cannot think of why, after all
>> 
>>   (macroexpand-all '`(,a ,b ,c)) => (list a b c)
>
> I think it's a question of style. As you, Philip, note downthread, I
> usually decide on whether I see [1] the context as "data with some
> interspersed (live) values" (think "json with variable interpolation"
> or as "program". Lisp itself is ambiguous about that :-)

In that case I agree that backquoting is more convenient, I was just
specifically referring to the case where every single element of a flat
list is unquoted or is self-evaluating.

> Cheers
>
> [1] and want to convey to my readers



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-16 19:46 ` Philip Kaludercic
  2022-12-16 21:29   ` Enrico Flor
@ 2022-12-17 14:17   ` Philip Kaludercic
  1 sibling, 0 replies; 11+ messages in thread
From: Philip Kaludercic @ 2022-12-17 14:17 UTC (permalink / raw)
  To: Enrico Flor; +Cc: emacs-devel

Philip Kaludercic <philipk@posteo.net> writes:

> +;; Why not use `memq'?
>  (defmacro latex-table-wizard--or (symbol &rest values)
>    "Return non-nil if SYMBOL is `eq' to one of VALUES."
>    (let ((bools (mapcar (lambda (value) `(eq ,symbol ,value))

To argue out this point, compare:

(disassemble (byte-compile '(or (eq foo 'a) (eq foo 'b) (eq foo 'c))))

--8<---------------cut here---------------start------------->8---
byte code:
  args: nil
0	varref	  foo
1	constant  a
2	eq	  
3	goto-if-not-nil-else-pop 1
6	varref	  foo
7	constant  b
8	eq	  
9	goto-if-not-nil-else-pop 1
12	varref	  foo
13	constant  c
14	eq	  
15:1	return	  
--8<---------------cut here---------------end--------------->8---

and:

(disassemble (byte-compile '(memq foo '(a b c))))

--8<---------------cut here---------------start------------->8---
byte code:
  args: nil
0	varref	  foo
1	constant  (a b c)
2	memq	  
3	return	  
--8<---------------cut here---------------end--------------->8---




^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [External] : Re: [NonGNU ELPA] New package: latex-table-wizard
  2022-12-17  6:22       ` tomas
  2022-12-17 10:01         ` Philip Kaludercic
@ 2022-12-17 16:13         ` Drew Adams
  1 sibling, 0 replies; 11+ messages in thread
From: Drew Adams @ 2022-12-17 16:13 UTC (permalink / raw)
  To: tomas@tuxteam.de, emacs-devel@gnu.org

> > > prefer `(,x ,y) over (list x y)
> 
> > I cannot think of why, after all
> > (macroexpand-all '`(,a ,b ,c)) => (list a b c)
> 
> I think it's a question of style. 

I use whichever I find most readable, case by case.

In some cases the former is clearer/simpler; in
other cases the latter is.  (To me, that is - I'm
the main reader of my code.)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-12-17 16:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-16 17:17 [NonGNU ELPA] New package: latex-table-wizard Enrico Flor
2022-12-16 19:46 ` Philip Kaludercic
2022-12-16 21:29   ` Enrico Flor
2022-12-16 21:37     ` Philip Kaludercic
2022-12-16 21:59       ` Enrico Flor
2022-12-16 22:58         ` Philip Kaludercic
2022-12-16 23:36           ` Enrico Flor
2022-12-17  6:22       ` tomas
2022-12-17 10:01         ` Philip Kaludercic
2022-12-17 16:13         ` [External] : " Drew Adams
2022-12-17 14:17   ` Philip Kaludercic

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.