auctex
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [AUCTeX] [PATCH] Add indentation for tabular environment


From: Tassilo Horn
Subject: Re: [AUCTeX] [PATCH] Add indentation for tabular environment
Date: Thu, 10 Oct 2013 16:30:23 +0200
User-agent: Gnus/5.130008 (Ma Gnus v0.8) Emacs/24.3.50 (gnu/linux)

Oleh <address@hidden> writes:

Hi Oleh,

> Here's the updated patch.
>
> One issue might be that the code assumes that each ampersand must be a
> column separator. But that is the case for all my LaTeX files.

Nice.

> I've also added a tests folder. The test specifies nicely what the
> code is supposed to do.

Yes, that probably not a bad idea to start collecting ERT tests.

Some nitpicks below:

> +(defun LaTeX-hanging-ampersand-position ()
> +  "Return indent position for a hanging (i.e. ^\\s-*&) ampersand."
> +  (destructuring-bind (beg-pos . beg-col)
> +      (LaTeX-env-beginning-pos-col)
> +    (let* ((cur-pos (point)))
> +      (save-excursion
> +        (if (re-search-backward "\\\\\\\\" beg-pos t)
> +            (let ((cur-idx (cl-count ?& (buffer-substring (point) cur-pos))))

`cl-count' is only available in Emacs 24, but we'd like to stay
compatible with at least Emacs 22 and also XEmacs 21.  (Using `count'
from `cl' also won't work, because we don't want to require that at
runtime.)

> +              (goto-char beg-pos)
> +              (re-search-forward "&" cur-pos t (+ 1 cur-idx))
> +              (- (current-column) 1))
> +          (+ 2 beg-col))))))
> +
> +(defconst LaTeX-tabularlike-end "\\\\end{\\(?:tabular\\|align\\)\\*?}")

I'd prefer that to be a list of tabular-like environment names, and then
you can build the regex where you need it using `regexp-opt'.  A list of
environment names has the benefit that style files (such as tabularx.el
and tabulary.el) can easily add their environments to that list in their
style hook.

> diff --git a/tests/latex/latex-test.el b/tests/latex/latex-test.el
> new file mode 100644
> index 0000000..89a8baf
> --- /dev/null
> +++ b/tests/latex/latex-test.el
> @@ -0,0 +1,17 @@
> +(defvar LaTeX-indent-tabular-test/in  (expand-file-name "tabular-in.tex"))
> +(defvar LaTeX-indent-tabular-test/out (expand-file-name "tabular-out.tex"))
> +
> +(ert-deftest LaTeX-indent-tabular ()
> +  (should (equal

I'd use `string-equal' here.

Other than the points above, the patch looks good.

Bye,
Tassilo




reply via email to

[Prev in Thread] Current Thread [Next in Thread]