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.