signature.ascDescription: PGP signature
|
| From: | Enrico Flor |
| Subject: | Re: [NonGNU ELPA] New package: latex-table-wizard |
| Date: | Fri, 16 Dec 2022 21:59:04 +0000 |
"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))))))))))
>>>
>>>
>>>
>>> @@ -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
signature.asc
Description: PGP signature
publickey - enrico@eflor.net - ffbd1445.asc
Description: application/pgp-keys
signature.asc
Description: OpenPGP digital signature
| [Prev in Thread] | Current Thread | [Next in Thread] |