comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Bob Rogers]

   First the fix (see the diff below):  When you type "C-c RET"
(comint-insert-input) in shell mode (e.g.) with point on a line of
output, nothing happens, not even a ding.  This seems to be because
comint-insert-input is trying to invoke the "RET" binding, but doesn't
allow for the fact that this-command-keys returns a string.  The patch
makes it work for me, though only if you define "working" as "insert a
newline at point".

   Hence the quibble:  I would argue that these bindings are backwards.
There have been many times that I've typed "RET" (comint-send-input) in
a shell buffer and mistakenly expected newline insertion, instead of the
defined "reinvoke this command" behavior.  In contrast, I never expect
insertion when I type "C-c RET"; besides which, the side-effects of an
accidental "C-c RET" are easier to undo.  So if "safety" were the reason
for changing comint-insert-input to work only on actual command input,
then it seems inconsistent not to do the same for comint-send-input as
well.  Moreover, I would argue that comint-send-input should be the
picky one, and comint-insert-input should be more forgiving, so that you
could then get the current effect of "RET" on an arbitrary line by
typing "C-c RET RET", i.e. insert at the process mark and then submit
it.  As it is, I see no way to get the former comint-copy-old-input
behavior, save by cut-and-paste.  Or have I missed something?

   If not, that's the bug:  I find it extremely useful to be able to
reinvoke lines of transcript output (e.g. commands echoed by "make")
after editing them, so it is frustrating that "C-c RET" no longer works
for that.  Even if the new behavior is deemed a feature (or gets
grandfathered due to release pressure), it amounts to an incompatible
change, but I can't find any mention of this in NEWS.  (Except for the
command name, the description in misc.texi hasn't changed, though the
new behavior does match it better than the old behavior did.)

   So the question is:  Misfeature, or documentation oversight?

   TIA,

					-- Bob Rogers
					   http://rgrjr.dyndns.org/

------------------------------------------------------------------------
Index: lisp/comint.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/comint.el,v
retrieving revision 1.340
diff -c -r1.340 comint.el
*** lisp/comint.el	22 Apr 2006 23:30:13 -0000	1.340
--- lisp/comint.el	6 May 2006 16:07:16 -0000
***************
*** 807,813 ****
      (if (not (eq (get-char-property (point) 'field) 'input))
  	;; No input at POS, fall back to the global definition.
  	(let* ((keys (this-command-keys))
! 	       (last-key (and (vectorp keys) (aref keys (1- (length keys)))))
  	       (fun (and last-key (lookup-key global-map (vector last-key)))))
  	  (goto-char pos)
  	  (and fun (call-interactively fun)))
--- 807,814 ----
      (if (not (eq (get-char-property (point) 'field) 'input))
  	;; No input at POS, fall back to the global definition.
  	(let* ((keys (this-command-keys))
! 	       (last-key (and (or (vectorp keys) (stringp keys))
! 			      (aref keys (1- (length keys)))))
  	       (fun (and last-key (lookup-key global-map (vector last-key)))))
  	  (goto-char pos)
  	  (and fun (call-interactively fun)))

comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 >    First the fix (see the diff below):  When you type "C-c RET"
 > (comint-insert-input) in shell mode (e.g.) with point on a line of
 > output, nothing happens, not even a ding.  

The name comint-insert-input suggests that it inserts (old) input so
to do nothing when the point isn't over input seems appropriate to me.

 >                                              This seems to be because
 > comint-insert-input is trying to invoke the "RET" binding, but doesn't
 > allow for the fact that this-command-keys returns a string.  

Its because comint-insert-input can also be invoked by mouse-2 which falls back
to the global binding (generally mouse-yank-at-click).

 >                                                              The patch
 > makes it work for me, though only if you define "working" as "insert a
 > newline at point".

A trivial fix is one thats simple to implement but this one doesn't seem
to give the right result.

 >    Hence the quibble:  I would argue that these bindings are backwards.
 > There have been many times that I've typed "RET" (comint-send-input) in
 > a shell buffer and mistakenly expected newline insertion, instead of the
 > defined "reinvoke this command" behavior.  

Yes this behaviour can have unforeseen consequences and my preference would
be to remove it.

 >                                            In contrast, I never expect
 > insertion when I type "C-c RET"; besides which, the side-effects of an
 > accidental "C-c RET" are easier to undo.  So if "safety" were the reason
 > ...

It isn't the reason.  We have tried to combine two commands with a
similar functionality (comint-insert-clicked-input, comint-copy-old-input)

 > for changing comint-insert-input to work only on actual command input,
 > then it seems inconsistent not to do the same for comint-send-input as
 > well.  Moreover, I would argue that comint-send-input should be the
 > picky one, and comint-insert-input should be more forgiving, so that you
 > could then get the current effect of "RET" on an arbitrary line by
 > typing "C-c RET RET", i.e. insert at the process mark and then submit
 > it.  As it is, I see no way to get the former comint-copy-old-input
 > behavior, save by cut-and-paste.  Or have I missed something?

I see now that comint-copy-old-input did indeed copy the whole line but
although the name would not suggest so.  Presumably sometimes you need
part of the line, in which case you would need to cut and paste anyway.

 >    If not, that's the bug:  I find it extremely useful to be able to
 > reinvoke lines of transcript output (e.g. commands echoed by "make")
 > after editing them, so it is frustrating that "C-c RET" no longer works
 > for that.  

Again the name comint-copy-old-input doesn't suggest this behaviour and
my preference would be not to have it.

 >           Even if the new behavior is deemed a feature (or gets
 > grandfathered due to release pressure), it amounts to an incompatible
 > change, but I can't find any mention of this in NEWS.  (Except for the
 > command name, the description in misc.texi hasn't changed, though the
 > new behavior does match it better than the old behavior did.)

That's right the behaviour you're describing wasn't documented.  You're
suggesting now that NEWS describe the removal of this undocumented
behaviour.  I think that might confuse people.

 >    So the question is:  Misfeature, or documentation oversight?

Its Richard's call.  If he wants comint-copy-old-input back I will reinstate
it.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   The name comint-insert-input suggests that it inserts (old) input so
   to do nothing when the point isn't over input seems appropriate to me.

Yes, but if comint-use-prompt-regexp is t, then comint-insert-input
does nothing even if point _is_ over input.

Just do `emacs -q -nbc', then
M-: (setq comint-use-prompt-regexp t)

M-x shell

umask RET

Go back to the umask on the previous prompt.

C-c RET.

Nothing happens.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

>From my previous response:

   Yes, but if comint-use-prompt-regexp is t, then comint-insert-input
   does nothing even if point _is_ over input.

On closer inspection, what seems to cause this bug is that
comint-insert-input contains:

    (if (not (eq (get-char-property (point) 'field) 'input))
    ;; No input at POS, fall back to the global definition.

If comint-use-prompt-regexp is non-nil, then there are no field
properties, so, despite what the comment claims, that condition does
_not_ assure that there is no input at POS.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Miles Bader]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:
> On closer inspection, what seems to cause this bug is that
> comint-insert-input contains:
>
>     (if (not (eq (get-char-property (point) 'field) 'input))
>     ;; No input at POS, fall back to the global definition.
>
> If comint-use-prompt-regexp is non-nil, then there are no field
> properties, so, despite what the comment claims, that condition does
> _not_ assure that there is no input at POS.

BTW, that function's use of fields is not well written.

Also, it uses `posn-set-point' _after_ getting the value of (point),
which doesn't make any sense to me.

The following seems to fix both problems (note new helper function following):


   (defun comint-insert-input (&optional event)
     "In a Comint buffer, set the current input to the previous input at point."
     ;; This doesn't use "e" because it is supposed to work
     ;; for events without parameters.
     (interactive (list last-input-event))
     (when event
       (posn-set-point (event-end event)))
     (let ((pos (point)))
       (if (not (eq (field-at-pos pos) 'input))
           ;; No input at POS, fall back to the global definition.
           (let* ((keys (this-command-keys))
                  (last-key (and (vectorp keys) (aref keys (1- (length keys)))))
                  (fun (and last-key (lookup-key global-map (vector last-key)))))
             (goto-char pos)
             (and fun (call-interactively fun)))
         (setq pos (point))
         ;; There's previous input at POS, insert it at the end of the buffer.
         (goto-char (point-max))
         ;; First delete any old unsent input at the end
         (delete-region
          (or (marker-position comint-accum-marker)
              (process-mark (get-buffer-process (current-buffer))))
          (point))
         ;; Insert the input at point
         (insert (field-string-no-properties pos)))))

   (defun field-at-pos (pos)
     "Return the field at position POS, taking stickiness etc into account"
     (let ((raw-field (get-char-property (field-beginning pos) 'field)))
       (if (eq raw-field 'boundary)
           (get-char-property (1- (field-end pos)) 'field)
         raw-field)))


I'll commit this unless somebody objects.


-Miles
-- 
"Though they may have different meanings, the cries of 'Yeeeee-haw!' and
 'Allahu akbar!' are, in spirit, not actually all that different."

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Stefan Monnier]

>    (defun field-at-pos (pos)
>      "Return the field at position POS, taking stickiness etc into account"
>      (let ((raw-field (get-char-property (field-beginning pos) 'field)))
>        (if (eq raw-field 'boundary)
>            (get-char-property (1- (field-end pos)) 'field)
>          raw-field)))

How does this work?
I don't see where the stickiness is tested/used.
Maybe we should export get_pos_property (in editfns.c) to elisp?


        Stefan

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Miles Bader]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>    (defun field-at-pos (pos)
>>      "Return the field at position POS, taking stickiness etc into account"
>>      (let ((raw-field (get-char-property (field-beginning pos) 'field)))
>>        (if (eq raw-field 'boundary)
>>            (get-char-property (1- (field-end pos)) 'field)
>>          raw-field)))
>
> How does this work?
> I don't see where the stickiness is tested/used.

`field-beginning' (and field-end) use stickiness info.

> Maybe we should export get_pos_property (in editfns.c) to elisp?

Perhaps so; what would be called?  `insertion-property'?

-Miles
-- 
.Numeric stability is probably not all that important when you're guessing.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   I see now that comint-copy-old-input did indeed copy the whole line but
   although the name would not suggest so.

What it did depended on the value of comint-use-prompt-regexp.  It
relied on the value of the variable comint-get-old-input, whose
default value was comint-get-old-input-default.  Here is the
emacs-21.3 doc of the latter function:

  Default for `comint-get-old-input'.
  If `comint-use-prompt-regexp-instead-of-fields' is nil, then either
  return the current input field, if point is on an input field, or the
  current line, if point is on an output field.
  If `comint-use-prompt-regexp-instead-of-fields' is non-nil, then return
  the current line with any initial string matching the regexp
  `comint-prompt-regexp' removed.

`comint-use-prompt-regexp-instead-of-fields' is now called
`comint-use-prompt-regexp'.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 >    I see now that comint-copy-old-input did indeed copy the whole line but
 >    although the name would not suggest so.
 > 
 > What it did depended on the value of comint-use-prompt-regexp.  It
 > relied on the value of the variable comint-get-old-input, whose
 > default value was comint-get-old-input-default.  Here is the
 > emacs-21.3 doc of the latter function:
 > 
 >   Default for `comint-get-old-input'.
 >   If `comint-use-prompt-regexp-instead-of-fields' is nil, then either
 >   return the current input field, if point is on an input field, or the
 >   current line, if point is on an output field.
 >   If `comint-use-prompt-regexp-instead-of-fields' is non-nil, then return
 >   the current line with any initial string matching the regexp
 >   `comint-prompt-regexp' removed.
 > 
 > `comint-use-prompt-regexp-instead-of-fields' is now called
 > `comint-use-prompt-regexp'.

There is also a comment by comint-use-prompt-regexp (also present in 21.3):

;; Note: If it is decided to purge comint-prompt-regexp from the source
;; entirely, searching for uses of this variable will help to identify
;; places that need attention.

I presume use of comint-prompt-regexp preceded the use of fields.
Perhaps this should be purged as there's no need to use two methods and this
would reduce the maintenance overhead.


--
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Bob Rogers]

   From: Nick Roberts <nickrob@snap.net.nz>
   Date: Mon, 8 May 2006 16:18:26 +1200

    >    I see now that comint-copy-old-input did indeed copy the whole line but
    >    although the name would not suggest so.
    > 
    > What it did depended on the value of comint-use-prompt-regexp.  It
    > relied on the value of the variable comint-get-old-input, whose
    > default value was comint-get-old-input-default.  Here is the
    > emacs-21.3 doc of the latter function:
    > 
    >   Default for `comint-get-old-input'.
    >   If `comint-use-prompt-regexp-instead-of-fields' is nil, then either
    >   return the current input field, if point is on an input field, or the
    >   current line, if point is on an output field.
    >   If `comint-use-prompt-regexp-instead-of-fields' is non-nil, then return
    >   the current line with any initial string matching the regexp
    >   `comint-prompt-regexp' removed.
    > 
    > `comint-use-prompt-regexp-instead-of-fields' is now called
    > `comint-use-prompt-regexp'.

So this *is* documented behavior.  Is the change in behavior
inadvertent, then?

   There is also a comment by comint-use-prompt-regexp (also present in 21.3):

   ;; Note: If it is decided to purge comint-prompt-regexp from the source
   ;; entirely, searching for uses of this variable will help to identify
   ;; places that need attention.

   I presume use of comint-prompt-regexp preceded the use of fields.
   Perhaps this should be purged as there's no need to use two methods and this
   would reduce the maintenance overhead.

Perhaps, but that doesn't address the original issue, namely that in
comint-mode "C-c RET" is now less useful that it had been.

   From: Miles Bader <miles.bader@necel.com>
   Date: Mon, 08 May 2006 13:49:19 +0900

   Luc Teirlinck <teirllm@dms.auburn.edu> writes:

   BTW, that function's use of fields is not well written.

   Also, it uses `posn-set-point' _after_ getting the value of (point),
   which doesn't make any sense to me.

   The following seems to fix both problems . . .

   I'll commit this unless somebody objects.

But it doesn't address the original problem:  "C-c RET" still does
nothing when invoked on an output line.  Please find a solution below
that merges your changes with the guts of the comint-copy-old-input
definition from 21.3, restoring the original behavior.  If that is
satisfactory to everyone, then I will undertake to ensure that all of
the documentation is consistent.

					-- Bob

------------------------------------------------------------------------
(defun comint-insert-input (&optional event)
  "In a Comint buffer, set the current input to the previous input at point."
  ;; This doesn't use "e" because it is supposed to work
  ;; for events without parameters.
  (interactive (list last-input-event))
  (when event
    (posn-set-point (event-end event)))
  (let* ((input-pos (point))
	 (insertion-point
	   (or (marker-position comint-accum-marker)
	       (let ((process (get-buffer-process (current-buffer))))
		 (if (not process)
		     (error "Current buffer has no process"))
		 (process-mark process))))
	 (string-to-insert
	   (if (eq (field-at-pos input-pos) 'input)
	       ;; Use the field.
	       (field-string-no-properties input-pos)
	       ;; No input at INPUT-POS, fall back to the "old input" heuristic.
	       (funcall comint-get-old-input))))
    ;; Insert the input, after first deleting any old unsent input at the end.
    (goto-char (point-max))
    (delete-region insertion-point (point))
    (insert string-to-insert)))

(defun field-at-pos (pos)
  "Return the field at position POS, taking stickiness etc into account"
  (let ((raw-field (get-char-property (field-beginning pos) 'field)))
    (if (eq raw-field 'boundary)
	(get-char-property (1- (field-end pos)) 'field)
      raw-field)))

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 >    There is also a comment by comint-use-prompt-regexp (also present in
 >    21.3):
 > 
 >    ;; Note: If it is decided to purge comint-prompt-regexp from the source
 >    ;; entirely, searching for uses of this variable will help to identify
 >    ;; places that need attention.
 > 
 >    I presume use of comint-prompt-regexp preceded the use of fields.
 >    Perhaps this should be purged as there's no need to use two methods and
 >    this would reduce the maintenance overhead.
 > 
 > Perhaps, but that doesn't address the original issue, namely that in
 > comint-mode "C-c RET" is now less useful that it had been.

You're quoting my reply to Luc.  My reply to you, which you appear to have
ignored, did address the original issue.

 >    From: Miles Bader <miles.bader@necel.com>
 >    Date: Mon, 08 May 2006 13:49:19 +0900
 > 
 >    Luc Teirlinck <teirllm@dms.auburn.edu> writes:
 > 
 >    BTW, that function's use of fields is not well written.
 > 
 >    Also, it uses `posn-set-point' _after_ getting the value of (point),
 >    which doesn't make any sense to me.
 > 
 >    The following seems to fix both problems . . .
 > 
 >    I'll commit this unless somebody objects.
 > 
 > But it doesn't address the original problem:  "C-c RET" still does
 > nothing when invoked on an output line.  Please find a solution below
 > that merges your changes with the guts of the comint-copy-old-input
 > definition from 21.3, restoring the original behavior.  If that is
 > satisfactory to everyone, then I will undertake to ensure that all of
 > the documentation is consistent.

I've already said why I don't like it.  We haven't reached an agreement
yet that this behaviour is desirable.


-- 
Nick                                           http://www.inet.net.nz/~nickrob

comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Bob Rogers]

   From: Nick Roberts <nickrob@snap.net.nz>
   Date: Tue, 9 May 2006 15:11:33 +1200

    > Perhaps, but that doesn't address the original issue, namely that in
    > comint-mode "C-c RET" is now less useful that it had been.

   You're quoting my reply to Luc.  My reply to you, which you appear to have
   ignored, did address the original issue.

At the time, it seemed more appropriate to reply indirectly; I now see
that that was a mistake.  My apologies.

    > But it doesn't address the original problem:  "C-c RET" still does
    > nothing when invoked on an output line.  Please find a solution below
    > that merges your changes with the guts of the comint-copy-old-input
    > definition from 21.3, restoring the original behavior.  If that is
    > satisfactory to everyone, then I will undertake to ensure that all of
    > the documentation is consistent.

   I've already said why I don't like it.  We haven't reached an agreement
   yet that this behaviour is desirable.

Yes, of course.  I only made an offer to follow up that was conditional
on acceptance.

   From: Nick Roberts <nickrob@snap.net.nz>
   Date: Mon, 8 May 2006 12:31:16 +1200

    >    First the fix (see the diff below):  When you type "C-c RET"
    > (comint-insert-input) in shell mode (e.g.) with point on a line of
    > output, nothing happens, not even a ding.  

   The name comint-insert-input suggests that it inserts (old) input so
   to do nothing when the point isn't over input seems appropriate to me.

The name comint-insert-input does suggest that it should be inserting
something, so I found "doing nothing" surprising.  At the very least, it
should explain why it is refusing do what I am obviously asking.

    >                                              This seems to be because
    > comint-insert-input is trying to invoke the "RET" binding, but doesn't
    > allow for the fact that this-command-keys returns a string.  

   Its because comint-insert-input can also be invoked by mouse-2 which
   falls back to the global binding (generally mouse-yank-at-click).

OK.  But why wouldn't this be a reasonable thing to do for keyboard
input as well?

    >                                            In contrast, I never expect
    > insertion when I type "C-c RET"; besides which, the side-effects of an
    > accidental "C-c RET" are easier to undo.  So if "safety" were the reason
    > ...

   It isn't the reason.  We have tried to combine two commands with a
   similar functionality (comint-insert-clicked-input, comint-copy-old-input)

So the loss of functionality was accidental, then?

    > for changing comint-insert-input to work only on actual command input,
    > then it seems inconsistent not to do the same for comint-send-input as
    > well.  Moreover, I would argue that comint-send-input should be the
    > picky one, and comint-insert-input should be more forgiving, so that you
    > could then get the current effect of "RET" on an arbitrary line by
    > typing "C-c RET RET", i.e. insert at the process mark and then submit
    > it.  As it is, I see no way to get the former comint-copy-old-input
    > behavior, save by cut-and-paste.  Or have I missed something?

   I see now that comint-copy-old-input did indeed copy the whole line but
   although the name would not suggest so.  Presumably sometimes you need
   part of the line, in which case you would need to cut and paste anyway.

Not necessarily.  It is often more convenient to copy the whole line,
then pare it down at the command prompt.

    >    If not, that's the bug:  I find it extremely useful to be able to
    > reinvoke lines of transcript output (e.g. commands echoed by "make")
    > after editing them, so it is frustrating that "C-c RET" no longer works
    > for that.  

   Again the name comint-copy-old-input doesn't suggest this behaviour and
   my preference would be not to have it.

That depends on how narrowly you want to define "input."

   . . .

   From: Nick Roberts <nickrob@snap.net.nz>
   Date: Tue, 9 May 2006 18:17:19 +1200

   Luc Teirlinck writes:
    > . . .
    > 
    > Was there any reason, other than merely a desire to combine two
    > somewhat similar commands, to get rid of comint-copy-old-input?
    > Was anything broken about it?

   I don't think so, but I made that change nearly two years ago.  Since then
   there hasn't exactly been a rush of bug reports, so I find it hard to
   believe that it's been that inconvenient.

Well, I found it within an hour of building emacs from CVS, having used
distro versions for many years now.  It's possible that I'm the only one
who ever uses C-c RET on output lines . . . but somehow I doubt that.

					-- Bob

comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 >    The name comint-insert-input suggests that it inserts (old) input so
 >    to do nothing when the point isn't over input seems appropriate to me.
 > 
 > The name comint-insert-input does suggest that it should be inserting
 > something, so I found "doing nothing" surprising.  At the very least, it
 > should explain why it is refusing do what I am obviously asking.

It could give a message "No input here".  I'll see if I can do that.

 >     >                                              This seems to be because
 >     > comint-insert-input is trying to invoke the "RET" binding, but doesn't
 >     > allow for the fact that this-command-keys returns a string.  
 > 
 >    Its because comint-insert-input can also be invoked by mouse-2 which
 >    falls back to the global binding (generally mouse-yank-at-click).
 > 
 > OK.  But why wouldn't this be a reasonable thing to do for keyboard
 > input as well?

It does.  The keyboard input in this case (C-c RET) doesn't have a global
binding: thats why it does nothing.

 >     >                                            In contrast, I never expect
 >     > insertion when I type "C-c RET"; besides which, the side-effects of an
 >     > accidental "C-c RET" are easier to undo.  So if "safety" were the reason
 >     > ...
 > 
 >    It isn't the reason.  We have tried to combine two commands with a
 >    similar functionality (comint-insert-clicked-input, comint-copy-old-input)
 > 
 > So the loss of functionality was accidental, then?

I can't remember but I don't find it unfortunate.

 >    Again the name comint-copy-old-input doesn't suggest this behaviour and
 >    my preference would be not to have it.
 > 
 > That depends on how narrowly you want to define "input."

Well, I would certainly exclude output from my definition. 

 > Well, I found it within an hour of building emacs from CVS, having used
 > distro versions for many years now.  It's possible that I'm the only one
 > who ever uses C-c RET on output lines . . . but somehow I doubt that.

The patch I've installed works on output lines when comint-use-prompt-regexp
is non-nil.  Does this do what you want?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   There is also a comment by comint-use-prompt-regexp (also present in 21.3):

   ;; Note: If it is decided to purge comint-prompt-regexp from the source
   ;; entirely, searching for uses of this variable will help to identify
   ;; places that need attention.

Note that it says "If" not "When".  In as far as I know there has
never been a decision made to plan to remove comint-use-prompt-regexp
in the future.

I believe that comint-use-prompt-regexp serves two purposes:

1.  A customizable option for people who do not like some of the
    unintuitive (to them) aspects of fields in Comint buffers, like
    unusual behavior of many motion commands.  (For instance paragraph
    commands in shell mode.)

2.  Potentially a matter of necessity should the heuristic for
    distinguishing input from output used when comint-use-prompt-regexp
    is nil malfunction.  In that case the Comint derived mode can fall
    back on using regeps to recognize prompts.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 >    There is also a comment by comint-use-prompt-regexp (also present in
 >    21.3):
 > 
 >    ;; Note: If it is decided to purge comint-prompt-regexp from the source
 >    ;; entirely, searching for uses of this variable will help to identify
 >    ;; places that need attention.
 > 
 > Note that it says "If" not "When".  In as far as I know there has
 > never been a decision made to plan to remove comint-use-prompt-regexp
 > in the future.

Yes, I understand the difference between "If" and "When".  Clearly a decision
hasn't been made because both the comment and variable are still there.  Its
just that if someone has suggested it might be done, there probably is a good
reason for doing it.  Since it was broken, now might have been a good time to
do it.  It was also written five or more years ago, during which the use of
fields has probably increased.

 > I believe that comint-use-prompt-regexp serves two purposes:
 > 
 > 1.  A customizable option for people who do not like some of the
 >     unintuitive (to them) aspects of fields in Comint buffers, like
 >     unusual behavior of many motion commands.  (For instance paragraph
 >     commands in shell mode.)
 > 
 > 2.  Potentially a matter of necessity should the heuristic for
 >     distinguishing input from output used when comint-use-prompt-regexp
 >     is nil malfunction.  In that case the Comint derived mode can fall
 >     back on using regeps to recognize prompts.

Since Miles has provided a patch that fixes the bug you pointed out, there
is no longer an immediate need to purge comint-prompt-regexp in any case


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   Since Miles has provided a patch that fixes the bug you pointed out, there
   is no longer an immediate need to purge comint-prompt-regexp in any case

I do not have the impression that Miles' patch fixes the bug with
comint-use-prompt-regexp that I pointed out.  Such a fix would require
using the variable comint-get-old-input again.

Note that `comint-insert-input' takes away the possibility that
derived modes had to adapt the behavior of `comint-copy-old-input' to
the peculiarities of the particular derived mode through the variable
comint-get-old-input, a possibility actually used in ielm buffers,
external Lisp buffers and Scheme buffers (and maybe by some external
packages).  This also breaks the similarity between RET
(comint-send-input) and C-RET, because RET still uses the variable
comint-get-old-input and thereby still respects mode-specific
implementations.

Was there any reason, other than merely a desire to combine two
somewhat similar commands, to get rid of comint-copy-old-input?
Was anything broken about it?

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

Luc Teirlinck writes:
 > Nick Roberts wrote:
 > 
 >    Since Miles has provided a patch that fixes the bug you pointed out, there
 >    is no longer an immediate need to purge comint-prompt-regexp in any case
 > 
 > I do not have the impression that Miles' patch fixes the bug with
 > comint-use-prompt-regexp that I pointed out.  Such a fix would require
 > using the variable comint-get-old-input again.

Well it fixes:

  Yes, but if comint-use-prompt-regexp is t, then comint-insert-input
  does nothing even if point _is_ over input.

I can't see that you pointed anything else out.

 > Note that `comint-insert-input' takes away the possibility that
 > derived modes had to adapt the behavior of `comint-copy-old-input' to
 > the peculiarities of the particular derived mode through the variable
 > comint-get-old-input, a possibility actually used in ielm buffers,
 > external Lisp buffers and Scheme buffers (and maybe by some external
 > packages).  

Why would they want to `adapt' comint-copy-old-input?  They seem to use
comint-get-old-input which *is* stll present in comint.el.

 >                This also breaks the similarity between RET
 > (comint-send-input) and C-RET, because RET still uses the variable
 > comint-get-old-input and thereby still respects mode-specific
 > implementations.

Yes, and I suggested making RET more like C-c RET since inadvertantly sending
output lines as input is inherently dangerous.

 > Was there any reason, other than merely a desire to combine two
 > somewhat similar commands, to get rid of comint-copy-old-input?
 > Was anything broken about it?

I don't think so, but I made that change nearly two years ago.  Since then
there hasn't exactly been a rush of bug reports, so I find it hard to
believe that it's been that inconvenient.


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   Well it fixes:

     Yes, but if comint-use-prompt-regexp is t, then comint-insert-input
     does nothing even if point _is_ over input.

Did you try it?  It does not work for me when I try it and I can not
see anything in Miles' code that could make it work.

   Why would they want to `adapt' comint-copy-old-input?  They seem to use
   comint-get-old-input which *is* stll present in comint.el.

But no longer called by comint-get-old-input's replacement function.

When taling about Miles' patch are we talking about the same thing?
I thought that you meant:

(defun comint-insert-input (&optional event)
  "In a Comint buffer, set the current input to the previous input at point."
	;; This doesn't use "e" because it is supposed to work
	;; for events without parameters.
	(interactive (list last-input-event))
	(when event
	  (posn-set-point (event-end event)))
	(let ((pos (point)))
	  (if (not (eq (field-at-pos pos) 'input))
	      ;; No input at POS, fall back to the global definition.
	      (let* ((keys (this-command-keys))
		     (last-key (and (vectorp keys) (aref keys (1- (length keys)))))
		     (fun (and last-key (lookup-key global-map (vector last-key)))))
		(goto-char pos)
		(and fun (call-interactively fun)))
	    (setq pos (point))
	    ;; There's previous input at POS, insert it at the end of the buffer.
	    (goto-char (point-max))
	    ;; First delete any old unsent input at the end
	    (delete-region
	     (or (marker-position comint-accum-marker)
		 (process-mark (get-buffer-process (current-buffer))))
	     (point))
	    ;; Insert the input at point
	    (insert (field-string-no-properties pos)))))

(defun field-at-pos (pos)
  "Return the field at position POS, taking stickiness etc into account"
  (let ((raw-field (get-char-property (field-beginning pos) 'field)))
    (if (eq raw-field 'boundary)
	(get-char-property (1- (field-end pos)) 'field)
      raw-field)))

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 >    Well it fixes:
 > 
 >      Yes, but if comint-use-prompt-regexp is t, then comint-insert-input
 >      does nothing even if point _is_ over input.
 > 
 > Did you try it?  It does not work for me when I try it and I can not
 > see anything in Miles' code that could make it work.

Yes, but not without restarting the shell.  I see now it just tidies up
comint-insert-input.


 >    Why would they want to `adapt' comint-copy-old-input?  They seem to use
 >    comint-get-old-input which *is* stll present in comint.el.
 > 
 > But no longer called by comint-get-old-input's replacement function.

What is comint-get-old-input's replacement function?

I've installed a patch which includes Miles's.  I called his new function
field-at-point rather than ield-at-pos for consistency with names like
posn-at-point and to avoid confusion with the more general meaning of
position.  Does this work as you would expect?

When comint-use-prompt-regexp is nil things should work as before.  When it
is t, you should get the old behaviour i.e output lines are copied too.

I would like to do the same for comint-send-input too.  At the moment, I can't
see an easy way to do it.


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

    > But no longer called by comint-get-old-input's replacement function.

   What is comint-get-old-input's replacement function?

I meant comint-copy-old-input's replacement function.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Miles Bader]

Nick Roberts <nickrob@snap.net.nz> writes:
> I've installed a patch which includes Miles's.  I called his new function
> field-at-point rather than ield-at-pos for consistency with names like
> posn-at-point and to avoid confusion with the more general meaning of
> position.

Please don't rename my functions without asking first.  I've restored
the proper name.

[The function doesn't refer to point at all...]

Thanks,

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

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 > > I've installed a patch which includes Miles's.  I called his new function
 > > field-at-point rather than ield-at-pos for consistency with names like
 > > posn-at-point and to avoid confusion with the more general meaning of
 > > position.
 > 
 > Please don't rename my functions without asking first.  I've restored
 > the proper name.
 >
 > [The function doesn't refer to point at all...]

Yes, you're right, posn-at-point should really be posn-at-pos.  You could
improve field-at-pos further by making the argument pos optional and defaulting
to point.  I won't do that because I can't be bothered to ask if its OK
to change `your' functions first although I assume we can all use them
without asking.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Miles Bader]

Nick Roberts <nickrob@snap.net.nz> writes:
> I won't do that because I can't be bothered to ask if its OK to change
> `your' functions first although I assume we can all use them without
> asking.

Christ, take a pill...

-Miles
-- 
Quidquid latine dictum sit, altum viditur.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 > Christ, take a pill...

If you just made the trivial change and explained why I wouldn't have to.

Now your change:

        * comint.el (comint-insert-input): Remove redundant calls to setq
        and goto-char.

looks good to me, but Stefan specifically added them on 2005-03-24:

	* comint.el (comint-insert-input): Obey mouse-yank-at-point.

Are you sure they are redundant?


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Miles Bader]

Nick Roberts <nickrob@snap.net.nz> writes:
> looks good to me, but Stefan specifically added them on 2005-03-24:
>
> 	* comint.el (comint-insert-input): Obey mouse-yank-at-point.
>
> Are you sure they are redundant?

Well with the current code, yes -- but note that the value of POS is
slightly different than it used to be in some cases because I moved the
call to posn-set-point before POS gets bound.

The call to `setq' seems clearly redundant (though in the old code it
would have been needed because the posn-set-point could have moved
point).

However now that I think about it, if EVENT is non-nil, the call to
`goto-char' in the old code would have moved point to _original_ value
of (point), not the value of (posn-point EVENT), and this is what FUN
would see.

That's I guess the right thing if `mouse-yank-at-point' is true, but
what if `mouse-yank-at-point' is nil?  How is the old code supposed to
work in that case?

[I have `mouse-yank-at-point' set to nil, and the new code seems to work
correctly for me, whereas the old code did not.]

Perhaps an explicit test of `mouse-yank-at-point' is needed?

-Miles
-- 
Freedom's just another word, for nothing left to lose   --Janis Joplin

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   When comint-use-prompt-regexp is nil things should work as before.  When it
   is t, you should get the old behaviour i.e output lines are copied too.

Now C-c RET works indeed OK if comint-use-prompt-regexp is non-nil,
but now mouse-2 gives problems.  I do not believe that mouse-2 should
do anything but yank if comint-use-prompt-regexp is non-nil, since the
behavior is too confusing otherwise, without any help echo to warn or
anything.  Of course, the mouse-2 help-echo should also be made
conditional on comint-use-prompt-regexp being nil.

Also, people who find the old behavior useful will now have to set
comint-use-prompt-regexp non-nil for no other reason than to restore
that behavior.  To me, this seems weird. 

You replaced comint-copy-old-input with comint-insert-input believing
that it was similar to comint-insert-clicked-input.  But it is not.
mouse-2 has an important global binding, yanking, and even overriding
that global binding for old input seems rather dubious, let alone
overriding it in the entire comint buffer, making yanking using
mouse-2 completely impossible.

On the other hand, `C-c RET' has no important global binding, so there
is no reason not to use it on output.  The user gets the opportunity
to edit, so even _if_ he hit `C-c RET' by accident, which is very
unlikely, no major harm is done.  So providing the extra functionality
to people who find it useful does not impose any real hardship on
people who never use it.

In certain derived Comint modes, like external Lisp mode, using `C-c RET'
on output seems even more often useful than in shell-mode, which,
if I understood correctly, is what Bob is concerned with.

I personally believe that restoring comint-copy-old-input would be a
good thing.  `comint-insert-input' has not been part of any release,
so if we ever plan to undo the change, now is the time to do it.

Regardless of whether comint-copy-old-input gets restored, I believe
that it would be good to make the mouse-2 binding and corresponding
help-echo conditional on comint-use-prompt-regexp being non-nil and
document that wherever the mouse-2 binding is mentioned.  I could do
that if there are no objections.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

 >    When comint-use-prompt-regexp is nil things should work as before.  When
 >    it is t, you should get the old behaviour i.e output lines are copied
 >    too.
 > 
 > Now C-c RET works indeed OK if comint-use-prompt-regexp is non-nil,
 > but now mouse-2 gives problems.  I do not believe that mouse-2 should
 > do anything but yank if comint-use-prompt-regexp is non-nil, since the
 > behavior is too confusing otherwise, without any help echo to warn or
 > anything.  Of course, the mouse-2 help-echo should also be made
 > conditional on comint-use-prompt-regexp being nil.

Luc, do you actually use comint?  You appear to know more about the variables
than me (perhaps from re-writing the manual) but I have been using mouse-2 to
insert input (in the GUD buffer) for a long time, and not found it confusing.

 > Also, people who find the old behavior useful will now have to set
 > comint-use-prompt-regexp non-nil for no other reason than to restore
 > that behavior.  To me, this seems weird. 

Fields provide the opportunity to distinguish between input and output:
I think we should use it.

 > You replaced comint-copy-old-input with comint-insert-input believing
 > that it was similar to comint-insert-clicked-input.  But it is not.
 > mouse-2 has an important global binding, yanking, and even overriding
 > that global binding for old input seems rather dubious, let alone
 > overriding it in the entire comint buffer, making yanking using
 > mouse-2 completely impossible.

What binding do you think comint-insert-clicked-input had?  It was mouse-2
and it overrode the global binding.  Perhaps you are also arguing against
that function which was introduced six years ago and in Emacs 21.

It doesn't make yanking impossible (try it), just yanking at previous input.

 > On the other hand, `C-c RET' has no important global binding, so there
 > is no reason not to use it on output.  The user gets the opportunity
 > to edit, so even _if_ he hit `C-c RET' by accident, which is very
 > unlikely, no major harm is done.  So providing the extra functionality
 > to people who find it useful does not impose any real hardship on
 > people who never use it.
 
Just like help-follow-mouse used to work on words which weren't
cross-references.

I think it lacks logic, adds complexity and just performs a task that can be
done with two mouse clicks anyway.

 > In certain derived Comint modes, like external Lisp mode, using `C-c RET'
 > on output seems even more often useful than in shell-mode, which,
 > if I understood correctly, is what Bob is concerned with.

If it is so useful, I wonder why it wasn't documented.  In fact the
documentation even suggests it wasn't intended to be used in this way.

 > I personally believe that restoring comint-copy-old-input would be a
 > good thing.  `comint-insert-input' has not been part of any release,
 > so if we ever plan to undo the change, now is the time to do it.

I think some of your assertions are wrong.

 > Regardless of whether comint-copy-old-input gets restored, I believe
 > that it would be good to make the mouse-2 binding and corresponding
 > help-echo conditional on comint-use-prompt-regexp being non-nil and
 > document that wherever the mouse-2 binding is mentioned.  I could do
 > that if there are no objections.

I don't even follow that, mouse-2 inserts input even when
comint-use-prompt-regexp being non-nil

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   Luc, do you actually use comint?

I use ielm extensively, I use GUD mode, ``M-x shell', inferior Lisp mode,
some Comint derived modes not included with the Emacs distribution and
probably some other Comint derived modes I am not immediately thinking
of right now.

   but I have been using mouse-2 to insert input (in the GUD buffer)
   for a long time, and not found it confusing.

I claimed that the mouse-2 behavior was confusing with your latest
changes applied and with comint-use-prompt-regexp non-nil.  How long
have you been using mouse-2 in this situation?  mouse-2 copies old
text to the current command line in the entire Comint buffer and never
yanks.  This is very confusing behavior to me.

    > Also, people who find the old behavior useful will now have to set
    > comint-use-prompt-regexp non-nil for no other reason than to restore
    > that behavior.  To me, this seems weird. 

   Fields provide the opportunity to distinguish between input and output:
   I think we should use it.

I was talking about people who wanted to use `C-c RET' to insert old
output at the current prompt.  Given the current code, people who want
that have to set comint-use-prompt-regexp non-nil and hence can not
use fields.  That is what seems weird to me.

   What binding do you think comint-insert-clicked-input had?  It was mouse-2
   and it overrode the global binding

Only on input.  But in the situation I described, it overwrites it in
the entire buffer.

   Perhaps you are also arguing against
   that function which was introduced six years ago and in Emacs 21.

I am not arguing to remove it now.  (I would have opposed it if I
would have been part of the discussion six years ago, but that is a
moot point now.)  It _never_ worked when comint-use-prompt-regexp was
non-nil and what I am arguing now is that, in fact, it should not work
in that situation, that is mouse-2 should have its global binding if
comint-use-prompt-regexp is non-nil.

   It doesn't make yanking impossible (try it), just yanking at previous input.

I _did_ try it, in the situation I described and it did make yanking
impossible in the entire buffer.
 
   I don't even follow that, mouse-2 inserts input even when
   comint-use-prompt-regexp being non-nil

But it does that in the entire buffer.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Miles Bader]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:
> I was talking about people who wanted to use `C-c RET' to insert old
> output at the current prompt.  Given the current code, people who want
> that have to set comint-use-prompt-regexp non-nil and hence can not
> use fields.  That is what seems weird to me.

Yes.  I think that by and large the user-visible behavior should be the
same regardless of what value `comint-use-prompt-regexp' has -- other
than the obvious consequences of using or not using fields of course
(e.g., the behavior of C-a).

I find comint-insert-input a confusing command, as its behavior is so
context dependent.

The current behavior of `C-c C-m' on an output field (when
comint-use-prompt-regexp is nil) seems not very useful to me; in a shell
buffer, for instance, it just does nothing.

For the keybinding case, at least, a simple behavior like "always insert
the current field (whether input or output) or line (when
comint-use-prompt-regexp is non-nil)" would seem a lot cleaner, easier to
explain, and more useful.

For the mouse-binding, in a shell buffer for instance, I think the same
simple behavior would also be better; perhaps for some modes, alternative
handling of output fields is better -- but maybe a cleaner way to handle
that would be to just bind other command to mouse-2?

-Miles
-- 
"Though they may have different meanings, the cries of 'Yeeeee-haw!' and
 'Allahu akbar!' are, in spirit, not actually all that different."

Re: comint-insert-input on non-command lines:

[Nick Roberts]

 > I claimed that the mouse-2 behavior was confusing with your latest
 > changes applied and with comint-use-prompt-regexp non-nil.  How long
 > have you been using mouse-2 in this situation?  mouse-2 copies old
 > text to the current command line in the entire Comint buffer and never
 > yanks.  This is very confusing behavior to me.

The first complaint was that C-c RET doesn't work everywhere, now you're
saying you don't like it because mouse-2 *does* work everyhere.

 >    Fields provide the opportunity to distinguish between input and output:
 >    I think we should use it.
 > 
 > I was talking about people who wanted to use `C-c RET' to insert old
 > output at the current prompt.  Given the current code, people who want
 > that have to set comint-use-prompt-regexp non-nil and hence can not
 > use fields.  That is what seems weird to me.

Thats how comint works: with comint-prompt-regexp or with fields.  Its not my
choosing.

Anyway, my position seems to be increasingly untenable so I've put
comint-copy-old-input back on C-c RET and comint-insert-input just on mouse-2.
As you are more enthusiastic about this arrangement than me, could you please
update the docs.  I don't intend to make any further changes to comint.el so
you are free to do so if you wish.


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

Nick Roberts wrote:

   The first complaint was that C-c RET doesn't work everywhere, now you're
   saying you don't like it because mouse-2 *does* work everyhere.

I have already explained that the difference is that mouse-2 has an
important global binding and `C-c RET' does not.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

Nick Roberts wrote:

   Anyway, my position seems to be increasingly untenable so I've put
   comint-copy-old-input back on C-c RET and comint-insert-input just
   on mouse-2.  As you are more enthusiastic about this arrangement
   than me, could you please update the docs.  I don't intend to make
   any further changes to comint.el so you are free to do so if you
   wish.

I will update the docs as soon as I am sure that Richard does not
object against reverting this change.  From Miles' response, I got the
impression that he does not object.  (I believe that Miles is still
the Comint maintainer.)

I believe that if we are not ready to just expunge
`comint-use-prompt-regexp' entirely, and I do not believe that we are,
then the only way of avoiding a non-nil value of
`comint-use-prompt-regexp' to lead to very different behavior not
directly related to field motion, was to revert the change.  The
difference would have been substantial in certain derived modes such
as Inferior Lisp mode.

Unless Richard or Miles objects, I will just make the comint-insert-input
binding of mouse-2 undefined if `comint-use-prompt-regexp' is non-nil,
since that function is field-specific and one can simply not make
yanking impossible in the entire buffer.

I would leave RET (comint-send-input) as it is.  It could be
considered somewhat dangerous, but it has had its current behavior for
ages now.  I will better document the behavior.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines:

[Richard Stallman]

    I will update the docs as soon as I am sure that Richard does not
    object against reverting this change.

This change shouldn't have been made now without discussion and my
approval.  Reverting it is the right thing to do.

    Unless Richard or Miles objects, I will just make the comint-insert-input
    binding of mouse-2 undefined if `comint-use-prompt-regexp' is non-nil,
    since that function is field-specific and one can simply not make
    yanking impossible in the entire buffer.

I want to see what Miles has to say before any change is made.

Re: comint-insert-input on non-command lines:

[Nick Roberts]

 >     I will update the docs as soon as I am sure that Richard does not
 >     object against reverting this change.
 > 
 > This change shouldn't have been made now without discussion and my
 > approval.  Reverting it is the right thing to do.

It might not have been the right change but it _was_ made after discussion and
with your approval:

              From: Richard Stallman <rms@gnu.org>
              To: Nick Roberts <nickrob@gnu.org>
              Cc: emacs-devel@gnu.org
              Subject: Re: [PATCH:revised] misc.texi, comint.el
              Date: Tue, 22 Jun 2004 19:17:48 -0400
              Status: O

              Please install it, unless others object soon.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

Richard Stallman wrote:

       I will update the docs as soon as I am sure that Richard does not
       object against reverting this change.

   This change shouldn't have been made now without discussion and my
   approval.  Reverting it is the right thing to do.

The change was made about two years ago with your approval, but it was
presented as eliminating code duplication without any change in
functionality.  Actually, from this thread I have the impression that
Nick was not even aware that his change involved a change in
functionality.  We discovered recently that there were several very
non-trivial changes in functionality, as well as bugs.  These changes
were never discussed, as people were not aware of them.  While Nick's
changes are nearly two years old, they were never part of any release,
so if we are going to revert them, then now is the time.

First of all, the change broke `C-c RET' completely when
comint-use-prompt-regexp is non-nil.  Secondly, it introduced
differences, in some derived modes substantial differences, between
`C-c RET' and RET, whereas the only difference is supposed to be that
`C-c RET' offers you a chance to edit the text that RET immediately
executes.  Some features of `C-c RET', which some people apparently
relied on, were eliminated.

The easiest way to deal with the above problems is to revert Nick's
changes, which Nick already partly did.  The work that remains to be
done if we continue this way is to better document the behavior of
`C-c RET', RET and mouse-2 in Comint buffers.

Because if comint-use-prompt-regexp is non-nil there are no fields in
the buffer, it is essentially impossible to implement something close
to Nick's `C-c RET' behavior if that variable is non-nil.

So the alternative to reverting Nicks change is: get rid of
comint-use-prompt-regexp entirely, something I believe we are not
ready to do, not now and maybe never; make RET behave like Nick's
`C-c RET'; put the old `C-c RET' and RET functionality on some
different keys, because people still want it (the old functionality is
especially powerful in external Inferior Lisp buffers); document all that.

The biggest and essentially irreversible decision in all this would be
the elimination of comint-use-prompt-regexp.  The amount of work would
be big and the risk would be big, all supposedly shortly before a release.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines:

[Richard Stallman]

    The easiest way to deal with the above problems is to revert Nick's
    changes, which Nick already partly did.  The work that remains to be
    done if we continue this way is to better document the behavior of
    `C-c RET', RET and mouse-2 in Comint buffers.

Let's proceed down that road.  Can you send a patch that would
correct the documentation about this?

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

Richard Stallman wrote:

       The easiest way to deal with the above problems is to revert Nick's
       changes, which Nick already partly did.  The work that remains to be
       done if we continue this way is to better document the behavior of
       `C-c RET', RET and mouse-2 in Comint buffers.

   Let's proceed down that road.  Can you send a patch that would
   correct the documentation about this?

I will do so, but there are various places that need changes.  Certain
features were not (or badly) documented, even before Nick's changes,
and they do need to be documented.  There are also a few minor code
changes involved.  So maybe it might take a few days.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines:

[Richard Stallman]

    I will do so, but there are various places that need changes.  Certain
    features were not (or badly) documented, even before Nick's changes,
    and they do need to be documented.  There are also a few minor code
    changes involved.  So maybe it might take a few days.

We can afford to wait a few days.

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

Here are finally my proposed code and doc changes.  The code change
removes the misleading mouse-face and mouse-2 help-echo over old input
when `comint-use-prompt-regexp' is non-nil, because mouse-2 just yanks
as usual in that case.  The change to shell.el is minor: it just makes
the shell-mode docstring use shell-mode-map, even if `C-h f
shell-mode' was not done from a shell buffer.

I can install if desired.

===File ~/comint.el-diff====================================
Index: comint.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/comint.el,v
retrieving revision 1.345
diff -c -r1.345 comint.el
*** comint.el	26 May 2006 16:44:53 -0000	1.345
--- comint.el	28 May 2006 02:00:53 -0000
***************
*** 1510,1532 ****
  				(concat input "\n")))
  
  	  (let ((beg (marker-position pmark))
! 	      (end (if no-newline (point) (1- (point))))
! 	      (inhibit-modification-hooks t))
  	    (when (> end beg)
! 	      ;; Set text-properties for the input field
! 	      (add-text-properties
! 	       beg end
! 	       '(front-sticky t
! 		 font-lock-face comint-highlight-input
! 		 mouse-face highlight
! 		 help-echo "mouse-2: insert after prompt as new input"))
  	      (unless comint-use-prompt-regexp
  		;; Give old user input a field property of `input', to
  		;; distinguish it from both process output and unsent
  		;; input.  The terminating newline is put into a special
  		;; `boundary' field to make cursor movement between input
  		;; and output fields smoother.
! 		(put-text-property beg end 'field 'input)))
  	    (unless (or no-newline comint-use-prompt-regexp)
  	      ;; Cover the terminating newline
  	      (add-text-properties end (1+ end)
--- 1510,1532 ----
  				(concat input "\n")))
  
  	  (let ((beg (marker-position pmark))
! 		(end (if no-newline (point) (1- (point))))
! 		(inhibit-modification-hooks t))
  	    (when (> end beg)
! 	      (add-text-properties beg end
! 				   '(front-sticky t
! 				     font-lock-face comint-highlight-input))
  	      (unless comint-use-prompt-regexp
  		;; Give old user input a field property of `input', to
  		;; distinguish it from both process output and unsent
  		;; input.  The terminating newline is put into a special
  		;; `boundary' field to make cursor movement between input
  		;; and output fields smoother.
! 		(add-text-properties
! 		 beg end
! 		 '(mouse-face highlight
! 		   help-echo "mouse-2: insert after prompt as new input"
! 		   field input))))
  	    (unless (or no-newline comint-use-prompt-regexp)
  	      ;; Cover the terminating newline
  	      (add-text-properties end (1+ end)
============================================================

===File ~/shell.el-diff=====================================
*** shell.el	26 May 2006 16:27:31 -0500	1.137
--- shell.el	27 May 2006 18:30:48 -0500	
***************
*** 367,373 ****
  (put 'shell-mode 'mode-class 'special)
  
  (define-derived-mode shell-mode comint-mode "Shell"
!   "Major mode for interacting with an inferior shell.
  \\[comint-send-input] after the end of the process' output sends the text from
      the end of process to the end of the current line.
  \\[comint-send-input] before end of process output copies the current line minus the prompt to
--- 367,373 ----
  (put 'shell-mode 'mode-class 'special)
  
  (define-derived-mode shell-mode comint-mode "Shell"
!   "Major mode for interacting with an inferior shell.\\<shell-mode-map>
  \\[comint-send-input] after the end of the process' output sends the text from
      the end of process to the end of the current line.
  \\[comint-send-input] before end of process output copies the current line minus the prompt to
============================================================

===File ~/misc.texi-diff====================================
*** misc.texi	17 May 2006 23:42:26 -0500	1.85
--- misc.texi	27 May 2006 20:55:01 -0500	
***************
*** 873,889 ****
  Move point to the following prompt (@code{comint-next-prompt}).
  
  @kindex C-c RET @r{(Shell mode)}
! @findex comint-insert-input
  @item C-c @key{RET}
  Copy the input command which point is in, inserting the copy at the end
! of the buffer (@code{comint-insert-input}).  This is useful if you
  move point back to a previous command.  After you copy the command, you
  can submit the copy as input with @key{RET}.  If you wish, you can
! edit the copy before resubmitting it.
  
  @item Mouse-2
! Copy the input command that you click on, inserting the copy at the end
! of the buffer.
  @end table
  
    Moving to a previous input and then copying it with @kbd{C-c
--- 873,893 ----
  Move point to the following prompt (@code{comint-next-prompt}).
  
  @kindex C-c RET @r{(Shell mode)}
! @findex comint-copy-old-input
  @item C-c @key{RET}
  Copy the input command which point is in, inserting the copy at the end
! of the buffer (@code{comint-copy-old-input}).  This is useful if you
  move point back to a previous command.  After you copy the command, you
  can submit the copy as input with @key{RET}.  If you wish, you can
! edit the copy before resubmitting it.  If you use this command on an
! output line, it copies that line to the end of the buffer.
  
  @item Mouse-2
! Copy the old input command that you click on, inserting the copy at the end
! of the buffer (@code{comint-insert-input}).  This command only works if
! @code{comint-use-prompt-regexp} is @code{nil} (the default).
! If @code{comint-use-prompt-regexp} is non-@code{nil} or if point is
! not over old input, @kbd{Mouse-2} just yanks as usual.
  @end table
  
    Moving to a previous input and then copying it with @kbd{C-c
============================================================

===File ~/inf-lisp-el-diff==================================
*** inf-lisp.el	10 Feb 2006 08:34:48 -0600	1.43
--- inf-lisp.el	27 May 2006 20:54:12 -0500	
***************
*** 232,262 ****
  documentation for variable `inferior-lisp-buffer'.
  
  \\{inferior-lisp-mode-map}
! 
! Customisation: Entry to this mode runs the hooks on `comint-mode-hook' and
  `inferior-lisp-mode-hook' (in that order).
  
  You can send text to the inferior Lisp process from other buffers containing
  Lisp source.
!     switch-to-lisp switches the current buffer to the Lisp process buffer.
!     lisp-eval-defun sends the current defun to the Lisp process.
!     lisp-compile-defun compiles the current defun.
!     lisp-eval-region sends the current region to the Lisp process.
!     lisp-compile-region compiles the current region.
  
      Prefixing the lisp-eval/compile-defun/region commands with
      a \\[universal-argument] causes a switch to the Lisp process buffer after sending
      the text.
  
  Commands:
! Return after the end of the process' output sends the text from the
      end of process to point.
! Return before the end of the process' output copies the sexp ending at point
      to the end of the process' output, and sends it.
! Delete converts tabs to spaces as it moves back.
! Tab indents for Lisp; with argument, shifts rest
      of expression rigidly with the current line.
! C-M-q does Tab on each line starting within following expression.
  Paragraphs are separated only by blank lines.  Semicolons start comments.
  If you accidentally suspend your process, use \\[comint-continue-subjob]
  to continue it."
--- 232,269 ----
  documentation for variable `inferior-lisp-buffer'.
  
  \\{inferior-lisp-mode-map}
! \\<inferior-lisp-mode-map>
! Customization: Entry to this mode runs the hooks on `comint-mode-hook' and
  `inferior-lisp-mode-hook' (in that order).
  
  You can send text to the inferior Lisp process from other buffers containing
  Lisp source.
!     `switch-to-lisp' switches the current buffer to the Lisp process buffer.
!     `lisp-eval-defun' sends the current defun to the Lisp process.
!     `lisp-compile-defun' compiles the current defun.
!     `lisp-eval-region' sends the current region to the Lisp process.
!     `lisp-compile-region' compiles the current region.
  
      Prefixing the lisp-eval/compile-defun/region commands with
      a \\[universal-argument] causes a switch to the Lisp process buffer after sending
      the text.
  
  Commands:
! \\[comint-send-input] after the end of the process' output sends the text from the
      end of process to point.
! \\[comint-send-input] before the end of the process' output copies the sexp ending at point
      to the end of the process' output, and sends it.
! \\[comint-copy-old-input] copies the sexp ending at point to the end of the process' output,
!     allowing you to edit it before sending it.
! If `comint-use-prompt-regexp' is nil (the default), \\[comint-insert-input] on old input
!    copies the entire old input to the end of the process' output, allowing
!    you to edit it before sending it.  If point is not on old input or if
!    `comint-use-prompt-regexp' is non-nil, \\[comint-insert-input] behaves according to
!    its global binding.
! \\[backward-delete-char-untabify] converts tabs to spaces as it moves back.
! \\[lisp-indent-line] indents for Lisp; with argument, shifts rest
      of expression rigidly with the current line.
! \\[indent-sexp] does \\[lisp-indent-line] on each line starting within following expression.
  Paragraphs are separated only by blank lines.  Semicolons start comments.
  If you accidentally suspend your process, use \\[comint-continue-subjob]
  to continue it."
============================================================

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

>From my previous message:

     @item Mouse-2
   ! Copy the old input command that you click on, inserting the copy at the end
   ! of the buffer (@code{comint-insert-input}).  This command only works if
   ! @code{comint-use-prompt-regexp} is @code{nil} (the default).
   ! If @code{comint-use-prompt-regexp} is non-@code{nil} or if point is
   ! not over old input, @kbd{Mouse-2} just yanks as usual.

Since we are talking about a mouse command, "if point is not over old input",
should be: "if the click is not over old input".

Sincerely,

Luc.

Re: comint-insert-input on non-command lines:

[Nick Roberts]

 > Here are finally my proposed code and doc changes.  The code change
 > removes the misleading mouse-face and mouse-2 help-echo over old input
 > when `comint-use-prompt-regexp' is non-nil, because mouse-2 just yanks
 > as usual in that case.  The change to shell.el is minor: it just makes
 > the shell-mode docstring use shell-mode-map, even if `C-h f
 > shell-mode' was not done from a shell buffer.

The change to comint.el looks good to me.

 >   @item Mouse-2
 > ! Copy the old input command that you click on, inserting the copy at the end
 > ! of the buffer (@code{comint-insert-input}).  This command only works if
 > ! @code{comint-use-prompt-regexp} is @code{nil} (the default).
 > ! If @code{comint-use-prompt-regexp} is non-@code{nil} or if point is
 > ! not over old input, @kbd{Mouse-2} just yanks as usual.

How about

 If @code{comint-use-prompt-regexp} is @code{nil} (the default), copy the old
 input command that you click on, inserting the copy at the end of the buffer
 (@code{comint-insert-input}).  If @code{comint-use-prompt-regexp} is
 non-@code{nil}, or if point is not over old input, just yank as usual.


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

Nick Roberts wrote:

   How about

    If @code{comint-use-prompt-regexp} is @code{nil} (the default),
    copy the old input command that you click on, inserting the copy
    at the end of the buffer (@code{comint-insert-input}).  If
    @code{comint-use-prompt-regexp} is non-@code{nil}, or if point is
    not over old input, just yank as usual.

That looks OK, except that, as I already pointed out in my second
message, "if point is not..." should probably be "if the click is
not...", because we are talking about a mouse command.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines:

[Luc Teirlinck]

I plan to install the patches I sent earlier if there are no further
objections or suggestions.  I changed the patch to misc.text to incorporate
Nick's suggestions:

===File ~/misc.texi-diff-b==================================
*** misc.texi	17 May 2006 23:42:26 -0500	1.85
--- misc.texi	30 May 2006 22:01:16 -0500	
***************
*** 873,889 ****
  Move point to the following prompt (@code{comint-next-prompt}).
  
  @kindex C-c RET @r{(Shell mode)}
! @findex comint-insert-input
  @item C-c @key{RET}
  Copy the input command which point is in, inserting the copy at the end
! of the buffer (@code{comint-insert-input}).  This is useful if you
  move point back to a previous command.  After you copy the command, you
  can submit the copy as input with @key{RET}.  If you wish, you can
! edit the copy before resubmitting it.
  
  @item Mouse-2
! Copy the input command that you click on, inserting the copy at the end
! of the buffer.
  @end table
  
    Moving to a previous input and then copying it with @kbd{C-c
--- 873,893 ----
  Move point to the following prompt (@code{comint-next-prompt}).
  
  @kindex C-c RET @r{(Shell mode)}
! @findex comint-copy-old-input
  @item C-c @key{RET}
  Copy the input command which point is in, inserting the copy at the end
! of the buffer (@code{comint-copy-old-input}).  This is useful if you
  move point back to a previous command.  After you copy the command, you
  can submit the copy as input with @key{RET}.  If you wish, you can
! edit the copy before resubmitting it.  If you use this command on an
! output line, it copies that line to the end of the buffer.
  
  @item Mouse-2
! If @code{comint-use-prompt-regexp} is @code{nil} (the default), copy
! the old input command that you click on, inserting the copy at the end
! of the buffer (@code{comint-insert-input}).  If
! @code{comint-use-prompt-regexp} is non-@code{nil}, or if the click is
! not over old input, just yank as usual.
  @end table
  
    Moving to a previous input and then copying it with @kbd{C-c
============================================================

Re: comint-insert-input on non-command lines:

[Bob Rogers]

   From: Luc Teirlinck <teirllm@dms.auburn.edu>
   Date: Tue, 30 May 2006 22:14:08 -0500 (CDT)

   I plan to install the patches I sent earlier if there are no further
   objections or suggestions.  I changed the patch to misc.text to incorporate
   Nick's suggestions:

I am happy with the result.  Thanks for taking care of this,

					-- Bob Rogers
					   http://rgrjr.dyndns.org/

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

>From my previous reply:

   This also breaks the similarity between RET (comint-send-input) and
   C-RET

Obviously, I meant the similarity between RET and `C-c RET'.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Luc Teirlinck]

Nick Roberts wrote:

   Again the name comint-copy-old-input doesn't suggest this behaviour and
   my preference would be not to have it.

Motivating the behavior of a function from its name seems backward.
One could always rename it to something like `comint-use-old-as-input'
or whatever.

There also is the possibility of keeping both comint-copy-old-input
(maybe renamed) and comint-insert-input (_without_ the latest
adaptation to non-nil comint-use-prompt-regexp).

`C-c RET' would then be bound to comint-copy-old-input by default and
mouse-2 to comint-insert-input (if comint-use-prompt-regexp is nil)
and one could tell people in the docs that if they prefer to have
`C-c RET' behave like mouse-2, they can rebind it to
comint-insert-input using comint-mode-hook.  Even when used on input,
there can be a difference in certain modes, such as Inferior Lisp mode.

Sincerely,

Luc.

Re: comint-insert-input on non-command lines: A trivial fix, a quibble, and a bug

[Nick Roberts]

Luc Teirlinck writes:
 > Nick Roberts wrote:
 > 
 >    Again the name comint-copy-old-input doesn't suggest this behaviour and
 >    my preference would be not to have it.
 > 
 > Motivating the behavior of a function from its name seems backward.
 > One could always rename it to something like `comint-use-old-as-input'
 > or whatever.

Its not backward because its not motivating the behaviour but presumably
reflecting the intended purpose.

 > There also is the possibility of keeping both comint-copy-old-input
 > (maybe renamed) and comint-insert-input (_without_ the latest
 > adaptation to non-nil comint-use-prompt-regexp).
 > ...

There are all kinds of possibilities.  Its just a bit sad that we always tussle
with obscure parts of code which few will use and never reach a release.

-- 
Nick                                           http://www.inet.net.nz/~nickrob