emacs-devel
[Top][All Lists]
Advanced

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

Re: cursor-in-non-selected-windows as buffer-local


From: Richard Stallman
Subject: Re: cursor-in-non-selected-windows as buffer-local
Date: Thu, 1 Nov 2001 11:56:06 -0700 (MST)

Your patch is basically good, but here are some comments on specific
points.

    +       doc: /* Returns the buffer-local value of SYMBOL in BUFFER.  
    +If SYMBOL does not have a buffer-local binding in BUFFER, it will
    +return the default binding of SYMBOL. */)

The convention for doc strings is to use the plain verb form,
without `s', and in the present tense.  Thus, it should look like this:

    +       doc: /* Return the buffer-local value of SYMBOL in BUFFER.  
    +If SYMBOL does not have a buffer-local binding in BUFFER,
    +return the default binding of SYMBOL. */)

However, SYMBOL is a bad name for this argument.  It says what the
expected data type is, but that is only a little helpful.  To be
really helpful, it should say what the value *means*.  This argument's
value is used as a variable, so VARIABLE is a better name.  That says
how the symbol is used, as well as that it is a symbol.

Also, it is not quite accurate to say it returns the buffer-local value,
since it sometimes returns the default value instead.

So it really should say this:

    +       doc: /* Returns the value of VARIABLE in BUFFER.  
    +If VARIABLE does not have a buffer-local binding in BUFFER, the
    +value is the default binding of VARIABLE. */)


    +     if ((idx == -1 || PER_BUFFER_VALUE_P (buf, idx))
    +         && SYMBOLP (PER_BUFFER_SYMBOL (offset)) &&
    +         EQ (PER_BUFFER_SYMBOL (offset), symbol)) 

* Always break the line before, not after, a binary operator
such as `&&'.

    +  cursor_non_selected = 
    +    !NILP (Fbuffer_local_value (intern ("cursor-in-non-selected-windows"),
    +                           w->buffer));

* You shouldn't call intern in a frequently executed place like this.
Instead, do it at initialization time (in the function syms_of_...),
and store the result in Qcursor_in_non_selected_windows.

* Always break the line before, not after, a binary operator
such as `='.



reply via email to

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