emacs-devel
[Top][All Lists]
Advanced

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

Re: address@hidden: octave-mod.el: wrong indentation for "IF", "FOR", e


From: John W. Eaton
Subject: Re: address@hidden: octave-mod.el: wrong indentation for "IF", "FOR", etc.]
Date: Thu, 11 Oct 2007 21:57:22 -0400

On 11-Oct-2007, Glenn Morris wrote:

| "John W. Eaton" wrote:
| 
| > 2007-10-10  John W. Eaton  <address@hidden>
| >
| >     * progmodes/octave-mod.el (octave-looking-at-kw,
| >     octave-re-search-forward-kw, octave-re-search-backward-kw):
| >     New functions.
| 
| Why so complicated - why not just let-bind case-fold search in the
| functions that need it?

I wanted to avoid repeating the same code in multiple places.  Isn't
that what subroutines/functions are for?  But in any case, my Emacs
Lisp skills are minimal.  That's one of the reasons I would like for
octave-*.el to live only in the Emacs distribution instead of also in
the Octave sources.  That way, maybe someone who knows Emacs Lisp
better than I can take over maintenance. So please fix it as you see
fit.

| > +(defun octave-re-search-forward-kw (regexp)
| > +  (let ((case-fold-search nil))
| > +    (re-search-forward regexp nil 'move inc)))
| > +
| > +(defun octave-re-search-backward-kw (regexp)
| > +  (let ((case-fold-search nil))
| > +    (re-search-backward regexp nil 'move inc)))
| 
| It seems ugly to (ab)use the variable `inc' in this way. If you do
| want to do it this way, wouldn't it be nicer to use something like:
| 
| (defun octave-re-search-forward (regexp &optional bound noerror count)
|   "Like `re-search-forward', but sets `case-fold-search' nil."
|   (let (case-fold-search)
|      (re-search-forward regexp bound noerror count)))

All the uses of these function are searching for kewords, which is why
I appended the -kw to the name.  I thought it best to simplify the
interface to only what is needed.  I missed inc, so yes, it should
also appear in the argument list.  But since the other two arguments
are always nil and 'move in this context, I don't see why they
need to be exposed in the argument list of the new functions.

jwe




reply via email to

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