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: [NonGNU ELPA] New package: latex-table-wizard Date: Fri, 16 Dec 2022 21:37:23 +0000 Message-ID: <87cz8j2fos.fsf@posteo.net> References: <87pmcjw9ly.fsf@eflor.net> <87h6xv2kts.fsf@posteo.net> <87mt7nvxz2.fsf@eflor.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="15779"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Enrico Flor Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Fri Dec 16 22:38:22 2022 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 1p6IPN-0003uG-M1 for ged-emacs-devel@m.gmane-mx.org; Fri, 16 Dec 2022 22:38:21 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1p6IOd-00031i-Pa; Fri, 16 Dec 2022 16:37:35 -0500 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 1p6IOU-0002zh-PH for emacs-devel@gnu.org; Fri, 16 Dec 2022 16:37:27 -0500 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 1p6IOR-0007SE-UP for emacs-devel@gnu.org; Fri, 16 Dec 2022 16:37:26 -0500 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id 77895240116 for ; Fri, 16 Dec 2022 22:37:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1671226641; bh=6wMMS/HC6t3P8LyaHI0841D+w20ehQJ0WAzq0bMvp6M=; h=From:To:Cc:Subject:Date:From; b=TfTT+dgAahhMw9m/HTgK5laM1XzedD+myiX9rKHCEa8cXM5DRyeGTADnjHExE58sX C8M8/NF1ICnfJ99lr8Bk0BGLXU01BzcIUiCDrDqLl//B5etVDCI2UoxbHR9B8veFFw IQgM6c9i2CIkVezfGCGiPbVQzG3QpUR5o1B1fO6vXGGBXmyh8aYi16d5caVRgwN307 Ox/RlW1K7sxas9jesh5N3dyEh2rV+g++nxheW/XZVmITU2nyKUrxl+P32yhk+brcij 0if53PTPAb4SurtdTX6QbqQ9SIrPszNXlUzBG4EUsBvAsJmwT9v9qj2yAdLWiIBTEZ gIj8cRKxIQK4A== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4NYj9r6pjZz6tqD; Fri, 16 Dec 2022 22:37:19 +0100 (CET) In-Reply-To: <87mt7nvxz2.fsf@eflor.net> (Enrico Flor's message of "Fri, 16 Dec 2022 21:29:14 +0000") 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_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham 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:301528 Archived-At: Enrico Flor 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 > 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" writes: > >> Enrico Flor 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 >>> 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)))))))))) >> >> >> >> @@ -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