[PATCH] Re: ox-latex table tabbing support.

[Daniel Fleischer <danflscr@gmail.com>]

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

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. 


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 4810 bytes --]

From b041dd62cbeea924ea6d2b6dee9b1142aef968ec Mon Sep 17 00:00:00 2001
From: Daniel Fleischer <danflscr@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


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

-- 

Daniel Fleischer