comint-carriage-motion, and option nomenclature

comint-carriage-motion, and option nomenclature

[Noah Friedman]

I have two remarks about the current implementation of
comint-carriage-motion.

The first is that currently, when I run a shell that has onlcr set on the
tty, I can't see any output in the buffer at all.  I think this is because
comint-carriage-motion needs to (goto-char start) before calling
skip-chars-forward to look for \r$.

Secondly, and more generally, I find the inconsistent usage of variables to
enable options "in the negative" vs. enabling "in the positive" to be
disturbing.  

Some user options are named positively, e.g. `enable-local-eval',
`enable-flow-control', `enable-multibyte-characters'.

On the other hand, some user options are named like
`inhibit-eol-conversion', `inhibit-startup-message', etc.

In a few cases (especially for temporarily disabling features that are
normally enabled by other flags, e.g. `inhibit-read-only' overrides the
effects of `buffer-read-only') this is reasonable; those are not user
options.

But I really think user options ought to be consistent: all options should
be of the form "enable-some-behavior".  It doesn't matter what the default
actually is; but options should not be named according to what the "normal"
code path is.  Rather they should indicate whether an activity that is
controlled by that particular option is currently enabled or not.

It's probably too late to change the variables I mentioned, but I'm making
an appeal that the variable `comint-inhibit-carriage-motion' be renamed to
`comint-enable-carriage-motion' and the default changed to t.  And that all
new user options in the future use "enable-", not "inhibit-".  

Re: comint-carriage-motion, and option nomenclature

[Miles Bader]

Noah Friedman <friedman@splode.com> writes:
> The first is that currently, when I run a shell that has onlcr set on the
> tty, I can't see any output in the buffer at all.  I think this is because
> comint-carriage-motion needs to (goto-char start) before calling
> skip-chars-forward to look for \r$.

I couldn't actually manage to reproduce the problem experimentally,
but I think you're right about the solution, so I changed it.

> It's probably too late to change the variables I mentioned, but I'm making
> an appeal that the variable `comint-inhibit-carriage-motion' be renamed to
> `comint-enable-carriage-motion' and the default changed to t.  And that all
> new user options in the future use "enable-", not "inhibit-".  

First, `comint-enable-carriage-motion' is not really intended to be a
user option, but something for derived modes to set.

Second, I think you're wrong about `enable-' vs. `inhibit-'.  There's a
tension between naming options to always refer to a "positive" action,
and naming options such that they refer to a change from the "normal"
situation.

I don't think there's any mechanical way to decide which is right, and
that we simply have to depend on programmer's judgement.  In the case
of the comint-carriage-motion, `comint-inhibit-carriage-motion' feels
right, whereas `comint-enable-carriage-motion' sounds wrong -- it
sounds like it refers to an exceptional case.

Obviously you disagree, but I guess that's the point: it's a matter of
taste and judgement, not something amenable to a simple rule.

-Miles
-- 
Occam's razor split hairs so well, I bought the whole argument!

Re: comint-carriage-motion, and option nomenclature

[Noah Friedman]

>I couldn't actually manage to reproduce the problem experimentally,
>but I think you're right about the solution, so I changed it.

Thanks.  On the other hand I could not find a case where I *couldn't*
reproduce the problem or I would have suggested a specific test case.  But
I'm glad the change makes sense anyway.

>Second, I think you're wrong about `enable-' vs. `inhibit-'.  There's a
>tension between naming options to always refer to a "positive" action,
>and naming options such that they refer to a change from the "normal"
>situation.

I agree that there may seem to be a psychologically preferable direction in
some cases, but I'm not convinced that that's a compelling rationale
anymore.

    * Your intuition may not match the user's.  My limited experience with
      usability experiments to date seem to suggest that the programmer is
      practically the least qualified individual to make usability
      decisions because they may be inclined to regard their own
      idiosyncrasies as natural and obvious, even if they're not.

    * What's going to happen if you decide later that the default should
      change?  Then the "normal" situation will be reversed; does this mean
      that the variable should be renamed, or will you introduce an
      inconsistency in the convention of ``nomenclature matches "normal"
      behavior''?

The advantage to picking a convention here is that both of these scenarios
are largely irrelevant--or at least in the case of making a poor usability
decision, the default can be reversed later without changing the names of
documented options and creating a flag day for preferences.

Of course, as you said we may just agree to disagree on this point.


As for comint-inhibit-carriage-motion, I looked again and see that you
didn't mark this as a user option, but I can immediately think of two
reasons why I might have had to touch that variable as a user.

The first is that I wrote a minor mode with which it interferes, because it
tries to do the same thing (using more conservative pattern matching
against specific programs that are known to overwrite themselves).  I'm not
sure if my minor mode should toggle your variable because that might
interfere with the major mode settings; so this might be a "programmer"
issue rather than a user one.  But for now I've been treating it as a user
variable.  And actually, I like your solution well enough that I think I
may abandon my minor mode anyway.

But the second reason is that there are badly-behaved programs out there.
For example, the `format' floppy disk formatter program typically found on
gnu/linux systems outputs a carriage return at the end of each output
sequence, instead of at the beginning.  So on a real terminal, the cursor
is always at the beginning of the line with status text appearing after it.
But in shell mode, I think comint-carriage-motion will just eat all the
output and you'll get no feedback at all.  So there may be specific cases
where you want to toggle that variable temporarily.  This was actually my
reason for using an program-output pattern matching filter in the first
place.

Re: comint-carriage-motion, and option nomenclature

[Miles Bader]

I've thought about this some more, and I think that my problem with your
suggestion is _not_ with changing the sense of the default value, but rather
the choice of variable name, in particular, the use of the verb `enable'.

A variable name containing `enable' is sort of the mirror image of one
containing `inhibit' -- it suggests that the normal state is `off', and that
you can set this variable to `on' to override that.  The reason is that both
of those verbs refer to the _change in state_ of the variable, so they're
essentially saying `setting this to true changes the state'.

Such variables are great for cases where there really is an obvious `normal'
state, which you sometimes want to override.  However, as you point out, the
carriage-motion case may not really fit this model.

By contrast, consider variable names like `comint-handle-carriage-motion' or
`comint-interpet-carriage' motion.  In these, the verb refers not to the
state change when you set the variable, but rather the action performed when
it's true.  To me, names like thisseem much more neutral, and better suited
to cases where either on or off are reasonable defaults.

I wouldn't object to using either of those names or something similar for
comint-carriage-motion, and changing the sense to positive == enabled.

In general, I think:

 (1) Variable names containing verbs like `enable' or `inhibit', which imply
     a change to the normal case, should only be used when there's an obvious
     default state.  Such variables should be named so that the normal state
     corresponds to a `nil' setting, and setting the variable to non-nil
     overrides this.

 (2) All other variables _shouldn't_ use verbs like `enable' or `inhibit'.

-Miles
-- 
P.S.  All information contained in the above letter is false,
      for reasons of military security.

Re: comint-carriage-motion, and option nomenclature

[Noah Friedman]

>By contrast, consider variable names like `comint-handle-carriage-motion' or
>`comint-interpet-carriage' motion.  In these, the verb refers not to the
>state change when you set the variable, but rather the action performed when
>it's true.  To me, names like thisseem much more neutral, and better suited
>to cases where either on or off are reasonable defaults.
>
>I wouldn't object to using either of those names or something similar for
>comint-carriage-motion, and changing the sense to positive == enabled.

I'd be in favor of that.

Oh, I also just noticed that comint-carriage-motion also handles backspace
motion, which is a fine idea but it makes the function more general than
its present name implies.  Maybe it should be called something like
comint-cursor-motion instead, so it will be the obvious place to add
additional motion handlers in the future.  In fact, this could be made more
customizable now.  What do you think of something like the following:

    (defvar comint-cursor-motion-table
      `(("\r$" . ,#'(lambda () (delete-char -1)))
        ("\r"  . ,#'(lambda () (delete-region (point) 
                                              (line-beginning-position))))
        ("\b"  . ,#'(lambda () (delete-char -2))))

    (defun comint-cursor-motion (start end)
      (save-excursion
        (goto-char start)
        (when (< (skip-chars-forward "^\b\r" end) (- end start))
          (save-match-data
            (save-restriction
              (widen)
              (let ((inhibit-field-text-motion t)
                    (buffer-read-only nil))
                (mapc #'(lambda (elt)
                          (goto-char start)
                          (while (re-search-forward (car elt))
                            (funcall (cdr elt))))
                      comint-cursor-motion-table)))))))

I'm not strongly advocating rewriting it this way, it was just an idea
that suggested itself, since I've seen other escape sequences from time to
time that might be interesting to handle (e.g. ^[[D instead of ^H).

> (1) Variable names containing verbs like `enable' or `inhibit', which imply
>     a change to the normal case, should only be used when there's an obvious
>     default state.  Such variables should be named so that the normal state
>     corresponds to a `nil' setting, and setting the variable to non-nil
>     overrides this.
>
> (2) All other variables _shouldn't_ use verbs like `enable' or `inhibit'.

A lot of variables today in fact don't use `enable' or `inhibit', and I
don't seem to find that confusing.  It's just the enable/inhibit contrast
that jumps out at me.

You've mostly convinced me; I'm just not sure how obvious what the default
states should be sometimes.  I looked over my own programs and noticed that
I never seem to use "enable" in variable names, but sometimes state
variables have "enabled" in their names, though.  Maybe the ambivalence is
just mine.  :-)

Re: comint-carriage-motion, and option nomenclature

[Miles Bader]

On Wed, Aug 28, 2002 at 02:40:33PM -0700, Noah Friedman wrote:
> I also just noticed that comint-carriage-motion also handles backspace
> motion, which is a fine idea but it makes the function more general than
> its present name implies.

Um, backspace _is_ a `carriage motion'.  There are more `motions' than just
`returning'...

-Miles
-- 
P.S.  All information contained in the above letter is false,
      for reasons of military security.