Re: Fwd: Revamp of rst.el for inclusion in GNU Emacs distribution.

[Glenn Morris <rgm@gnu.org>]

"Martin Blais" wrote:

> http://svn.berlios.de/svnroot/repos/docutils/trunk/docutils/tools/editors/emacs/rst.el
[...]
> Any comments?

I know nothing about ReStructuredText, but the code looks basically ok.

Don't define functions with generic names like `filter' and
`line-number-at-pos'. Instead, define `rst-filter' etc that do what
you want.

Since jit-lock has been around since Emacs 21, the messing with
font-lock-support in rst-mode is unnecessary.

`rst-mode-lazy' seems like a poor name for what the variable does
(toggle extra highlighting), and again given that jit lock has been
around since Emacs 21 it could probably just be dumped.

I'd put the defcustoms and defgroup all together at the start of the
file, so people can easily see what can be customized.

The :types of defcustoms should just be 'hook, etc, not '(hook), AFAIK.

You need

(eval-when-compile
  (require 'cl))

for flet to work, but couldn't you just define an actual function
rst-addnum?

You need to get rid of `position' in rst-straighten-decorations, since
it's a CL function which should not be called at runtime.

Byte compiling reveals some free variables, all (?) caused by things
being defined after they are used: rst-toc-insert-max-level,
rst-toc-buffer-name, rst-toc-return-buffer, rst-level-face-max,
rst-level-face-format-light, rst-level-face-base-color,
rst-level-face-step-light, rst-level-face-base-light

In rst-replace-lines, p and l should be let-bound.

Why isn't rst-portable-mark-active-p just checking `mark-active' in
the Emacs case? Like all functions, the definition should be moved
before its first use.

> What's the next step?

For all the copyright holders to be willing to sign assignments.