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 <something else>) > ...) > > just do > > (let ((x <something else>)) > ...) > > 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 <danfl...@gmail.com> 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 &optional 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. + (separator + (format "\\hspace{%s\\textwidth} \\= " + (- (/ 1.0 count) 0.01)))) + (concat (apply 'concat (make-list count separator)) + "\\kill")))) (defun org-latex--decorate-table (table attributes caption above? info) "Decorate TABLE string with caption and float environment. @@ -3836,21 +3834,19 @@ (defun org-latex--org-table (table contents info) (org-latex--decorate-table output attr caption above? info)))))) -(defun org-table--org-tabbing (table contenst info) - "Return appropriate LaTeX code for an Org table, using the -latex tabbing syntax. +(defun org-table--org-tabbing (table contents info) + "Return tabbing environment latex code for Org table. TABLE is the table type element to transcode. CONTENTS is its contents, as a string. INFO is a plist used as a communication channel. + This function assumes TABLE has `org' as its `:type' property and `tabbing' as its `:mode' attribute." - (let ((output (format "\\begin{%s}\n%s\n%s\\end{%s}" - "tabbing" - (org-latex--align-string-tabbing table info ) - contenst - "tabbing"))) - output) - ) + (format "\\begin{%s}\n%s\n%s\\end{%s}" + "tabbing" + (org-latex--align-string-tabbing table info) + contents + "tabbing")) (defun org-latex--table.el-table (table info) "Return appropriate LaTeX code for a table.el table. -- 2.36.0
-- Daniel Fleischer
