Re: gas-mode.el - Comments welcome

[Stefan Monnier <monnier@iro.umontreal.ca>]

> It is now time time to check it out in order to make it ready for
> inclusion with GNU Emacs.  So I'm asking you for comments.  If you do
> assembly language programming, you could try it out.  If you know
> someone who might be interested, you could make him aware of it.
> Also, since Lisp is not the language I usually do much programming in,
> tips, hints and comments on coding style and conformance to coding
> conventions are welcome.

I don't do much asm programming (just enough to prompt me to slightly revise
asm-mode.el's indentation a few years while back), but here are some
comments:
- when you define faces used for font-locking, try to make them inherit from
  font-lock's default faces.  I personally hate color-based highlighting and
  use font-based highlighting instead, and every mode which redefines its
  own set of faces is just a pain in the <beep>, unless it does it by
  inheriting from the default faces.
- I'd use word and symbol syntaxes instead of `gas-re-sym' and friends.
  I.e. instead of (skip-chars-forward gas-skip-sym), I'd use
  (skip-syntax-forward "w_") and I'd use "\\_>" and "\\_<" for symbol
  boundaries instead of gas-re-nosym.
- it seems that comment-start, comment-start-skip, comment-end,
  comment-end-skip are not defined.
- try C-u M-x checkdoc-current-buffer RET
  It'll complain about things which you may rightfully decide to leave
  unchanged, but it will also complain about some cosmetic things which you
  need to fix, e.g. the first line should have 3 semi-colons rather than 2.
  It may also tell you to use names such as "*-flag" for boolean vars, with
  which I completely disagree.
- add some ;;;###autoload cookies (at least one for the major mode).
- fix naming to use the `gas-' (or `asm-') prefix for vars such as
  `line-cache'.  Provide defvars with docstring explaning what
  they contain.
- gas-change-comment-string seems not to be used.
- your syntax-table also accepts comments of the form //...\n although this
  seems to be mentioned nowhere.  Is it done on purpose or is it
  an oversight?
- not sure if I like the "use forward-sexp to jump to next occurrence of
  highlighted symbol".  In conjunction with C-M-t for example, this is odd.
  The intention of forwrd-sexp-function is to allow the use of elisp code
  when the syntax-table isn't flexible enough to describe some "sexps"
  (e.g. sexps like "begin ... end").  If you want to extend the semantics of
  C-M-f (rather than forward-sexp which may very well be called
  non-interactively by other pieces of Elisp code) then rebind this
  key sequence.
- The docstring of gas-mode says that gas-comment calls comment-dwim, but
  I don't see where that happens.  Also its description of comment-dwim
  doesn't quite match my understanding of it.  AFAIK it doesn't treat
  "comment with leading white space" specially and it doesn't remove
  comments unless provided with a C-u prefix.
- Try to present it as a patch to asm-mode.el (this one does set
  comment-start and friends, so the patch would make it trivial to see that
  this part was incorrectly removed).


        Stefan