Kyle Meyer [2022-06-25 Sat 23:49] wrote:
> Thanks for flagging this, Ihor. I was just glancing at this commit
> (4a0d951c6) due to seeing this warning. It's doing
>
> (let ((align ...))
> (setq align ...))
>
> where the align value is returned, so the align binding can be dropped
> altogether.
>
> Daniel, in addition to that, there are at least a few other issues with
> 4a0d951c6 that should be addressed:
>
> * the first line of the new docstrings should be a complete sentence.
>
>For example
>
> Return an appropriate LaTeX alignment string, for the
> latex tabbing environment.
>
>should be changed to something like
>
> Return alignment string for LaTeX tabbing environment.
>
>See (info "(elisp)Documentation Tips")
>
> * the indentation is off in several places, including the start of the
>docstrings. Please indent the code with, e.g., indent-region.
>
> * one of org-table--org-tabbing's parameters is a typo (contenst),
>which it looks like Ihor pointed out in an earlier review
>
> * comment typo: documets
>
> * several spots put opening and trailing parentheses on their own line
>
>That goes against the usual conventions of this repo:
>
> $ git grep '^ *)' '*.el' | wc -l
> 42
> $ git grep '( *$' '*.el' | wc -l
> 17
>
> * rather than doing something like
>
> (let ((x ""))
>(setq x )
>...)
>
>just do
>
> (let ((x ))
> ...)
>
>And consider whether it's worth adding a binding at all rather than
>inlining the code.
>
>As an extreme case, org-table--org-tabbing does
>
> (let ((output (format ...)))
>output)
>
>rather than
>
> (format ...)
Thanks for the code feedback; patch attached.
>From b041dd62cbeea924ea6d2b6dee9b1142aef968ec Mon Sep 17 00:00:00 2001
From: Daniel Fleischer
Date: Sun, 26 Jun 2022 17:25:00 +0300
Subject: [PATCH] lisp/ox-latex.el: tabbing code refactor
* lisp/ox-latex.el: documentation, indentation, cleaning
(org-latex-table)
(org-latex--align-string-tabbing)
(org-table--org-tabbing)
See
https://lists.gnu.org/archive/html/emacs-orgmode/2022-06/msg00700.html.
---
lisp/ox-latex.el | 66 +++-
1 file changed, 31 insertions(+), 35 deletions(-)
diff --git a/lisp/ox-latex.el b/lisp/ox-latex.el
index 898fa34dd..1446b7fca 100644
--- a/lisp/ox-latex.el
+++ b/lisp/ox-latex.el
@@ -3667,7 +3667,8 @@ (defun org-latex-table (table contents info)
((or (string= type "math") (string= type "inline-math"))
(org-latex--math-table table info))
;; Case 3: Tabbing
- ((string= type "tabbing") (org-table--org-tabbing table contents info))
+ ((string= type "tabbing")
+(org-table--org-tabbing table contents info))
;; Case 4: Standard table.
(t (concat (org-latex--org-table table contents info)
;; When there are footnote references within the
@@ -3706,32 +3707,29 @@ (defun org-latex--align-string (table info math?)
(apply 'concat (nreverse align)
(defun org-latex--align-string-tabbing (table info)
-"Return an appropriate LaTeX alignment string, for the
-latex tabbing environment.
+ "Return LaTeX alignment string using tabbing environment.
TABLE is the considered table. INFO is a plist used as
a communication channel."
-(or (org-export-read-attribute :attr_latex table :align)
-(let ((align "")
- (count 0)
- (separator ""))
-;; Count the number of cells in the first row.
-(setq count (length
- (org-element-map
- (org-element-map table 'table-row
-(lambda (row)
- (and (eq (org-element-property :type row) 'standard) row))
-info 'first-match)
- 'table-cell
-(lambda (cell) cell
-;; Calculate the column width, using a proportion of the documets
-;; textwidth.
-(setq separator (format
- "\\hspace{%s\\textwidth} \\= "
- (- (/ 1.0 count) 0.01)))
-(setq align (concat
- (apply 'concat (make-list count separator))
- "\\kill")))
-))
+ (or (org-export-read-attribute :attr_latex table :align)
+ (let* ((count
+ ;; Count the number of cells in the first row.
+ (length
+ (org-element-map
+ (org-element-map table 'table-row
+ (lambda (row)
+ (and (eq (org-element-property :type row)
+'standard)
+row))
+ info 'first-match)
+ 'table-cell
+ (lambda (cell) cell
+ ;; Calculate the column width, using a proportion of
+ ;;the documets textwidth.
+
