Re: major mode for fpscalc + some meta

[Stefan Monnier]

> I wanted to share this with you: I made a major mode for
> the tool fpscalc. fpscalc (fps = fixed priority

There's gnu.emacs.sources specifically for these kinds of announcements.

> serious people, so I would be very appreciative if you
> gave me feedback.

See below,


        Stefan


> ;;;; fpscalc-mode

Try C-u M-x checkdoc-current-buffer RET.  It'll help you follow some of
the usual conventions.

> (defface fpscalc-program-parts-face
>   '((t :background "black" :foreground "magenta" :bold t))
>   "The words declarations, initialise, semaphores, and formulas."
>   :group 'fpscalc-faces)

You can remove the "-face" suffix.
You need to (defgroup 'fpscalc-faces ...) first.
After that, you can remove the ":group 'fpscalc-faces".

> (defvar fpscalc-program-parts-face 'fpscalc-program-parts-face)

Please throw this away.  It just helps to confuse people.

> (defface fpscalc-comment-face
>   '((t :background "black" :foreground "blue" :bold t))
>   "The comment sign (!) as well as everything to its right."
>   :group 'fpscalc-faces)

I can live with new faces for some perceived customizability need, but
don't ignore the user's pre-existing preferences.  I.e. make your faces
inherit from the corresponding font-lock face, so that users can
customize their faces specifically for your mode, but they don't have to.

> (add-to-list 'auto-mode-alist  '("\\.fps\\'"  . fpscalc-mode))

Good.  You probably want to add a ";;;###autoload" cookie in front, in
case you want to distribute your file via ELPA, for example.

> (setq fpscalc-keywords

`setq' is for modifying the value of a previously declare variable.
Use `defvar' here, please.

> (define-derived-mode fpscalc-mode c-mode
>   "fpscalc-mode is a major mode for the tool fpscalc."

The third arg of define-derived-mode is not the docstring but the mode's name.

>   (setq mode-name "fpscalc")

Once you fix the previous problem, you can get rid of this.

>   (set-variable 'line-number-mode t t)

Why?  It's enabled by default anyway, and I can't think of any reason
why fpscalc-mode would need to enable it even if the user decided to
disable it globally.

Oh, and don't call `set-variable' from Elisp.  It's a command meant for M-x.

>   (abbrev-mode 0) ; disable (gets it from the C mode)

Why should you disable it?  I think what you want to do here instead is
to get rid of c-mode's abbreviations (i.e. setup fpscalc-mode's own
abbrev-table).

>   :syntax-table fpscalc-syntax-table

This has no effect, because it would only have an effect if it were
right after the docstring.  In any case, the right way to do it is to
name your syntax-table `fpscalc-mode-syntax-table' and to place its
definition before define-derived-mode.

>   (define-key fpscalc-mode-map [remap comment-dwim] 'fpscalc-comment-dwim)

Same for fpscalc-mode-map: define it once and for all before the
define-derived-mode.

> (defun fpscalc-comment-dwim (cmt)
>   (interactive "*P")
>   (require 'newcomment)
>   (let ((comment-start "!") (comment-end ""))
>     (comment-dwim cmt) ))

Don't do that.  Just do

   (set (make-local-variable 'comment-start) "!")
   (set (make-local-variable 'comment-end) "")

in fpscalc-mode.

> (defvar fpscalc-syntax-table nil "The syntax table for `fpscalc-mode'.")
> (setq fpscalc-syntax-table
>   (let ((syntax-table (make-syntax-table)))
>     (modify-syntax-entry ?! "< b" syntax-table)
>     (modify-syntax-entry ?\n "> b" syntax-table)
>     syntax-table) )

Merge the two:

(defvar fpscalc-mode-syntax-table
  (let ((syntax-table (make-syntax-table)))
    (modify-syntax-entry ?! "< b" syntax-table)
    (modify-syntax-entry ?\n "> b" syntax-table)
    syntax-table)
  "The syntax table for `fpscalc-mode'.")

> (defconst *path-of-fpscalc* "/home/incal/dvk/rt/fpscalc/fpscalc")

That should probably be turned into

  (defcustom fpscalc-command-name "fpscalc"
    "Name of the `fpscalc' executable to use."
    :type 'file)

> (defun compute ()
>   (interactive)
>   (let*((src (buffer-file-name))
>         (dst (format "%s_fallout.txt" src)))
>     (shell-command (format "%s < %s > %s" *path-of-fpscalc* src dst))

If `src' includes funny characters (like semi-colons, dollars, ...),
this will not do what you intended: you should pass `src' and `dst'
through shell-quote-argument.

>     (find-file dst) ))

You might prefer (display-buffer (find-file-noselect dst)), so that it
doesn't always replace the source buffer, but can display the output in
another window.


        Stefan