[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] Re: ox-latex table tabbing support.
From: |
Daniel Fleischer |
Subject: |
[PATCH] Re: ox-latex table tabbing support. |
Date: |
Sun, 26 Jun 2022 17:46:15 +0300 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (darwin) |
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.
0001-lisp-ox-latex.el-tabbing-code-refactor.patch
Description: patch
--
Daniel Fleischer
- Re: ox-latex table tabbing support., (continued)
- Re: ox-latex table tabbing support., Daniel Fleischer, 2022/06/24
- Re: ox-latex table tabbing support., Robert Pluim, 2022/06/24
- Re: ox-latex table tabbing support., Daniel Fleischer, 2022/06/24
- Re: ox-latex table tabbing support., emacs, 2022/06/24
- Re: ox-latex table tabbing support., Ihor Radchenko, 2022/06/24
- Re: ox-latex table tabbing support., Robert Pluim, 2022/06/26
- [PATCH] describe how to override Author, Robert Pluim, 2022/06/26
- Re: [PATCH] describe how to override Author, Daniel Fleischer, 2022/06/26
Re: ox-latex table tabbing support., Ihor Radchenko, 2022/06/25