using pcomplete for comint.el/shell.el

using pcomplete for comint.el/shell.el

[Colin Walters]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

John Weigley has been having trouble with getting papers signed for his
general Emacs contributions, so he asked me to finish the last bit of
work needed to get shell.el to use pcomplete.el.  

One of the design goals was to avoid loading pcomplete unless the user
actually used the completion facilities.  So this made things a little
bit tricky.  I've attached the patch to shell.el to implement this. 
Other modes will have to do something very similar.  

Or would it be better if we just unconditionally required pcomplete.el
for shell.el?

After this is implemented, we can just remove a lot of cruft in
comint.el.  

Also, one issue to consider is that this will break backwards
compatibility with modes deriving from comint.




[-- Attachment #2: shell.diff --]
[-- Type: text/plain, Size: 1956 bytes --]

--- shell.el.~1.105.~	Mon Dec  3 12:24:41 2001
+++ shell.el	Thu Feb 14 18:49:01 2002
@@ -310,7 +310,8 @@
        (setq shell-mode-map (nconc (make-sparse-keymap) comint-mode-map))
        (define-key shell-mode-map "\C-c\C-f" 'shell-forward-command)
        (define-key shell-mode-map "\C-c\C-b" 'shell-backward-command)
-       (define-key shell-mode-map "\t" 'comint-dynamic-complete)
+       (define-key shell-mode-map "\t" 'shell-pcomplete)
+       (define-key shell-mode-map "\M-\t" 'shell-pcomplete-reverse)
        (define-key shell-mode-map "\M-?"
 	 'comint-dynamic-list-filename-completions)
        (define-key shell-mode-map [menu-bar completion]
@@ -396,7 +397,6 @@
   (setq comint-delimiter-argument-list shell-delimiter-argument-list)
   (setq comint-file-name-chars shell-file-name-chars)
   (setq comint-file-name-quote-list shell-file-name-quote-list)
-  (setq comint-dynamic-complete-functions shell-dynamic-complete-functions)
   (make-local-variable 'paragraph-start)
   (setq paragraph-start comint-prompt-regexp)
   (make-local-variable 'font-lock-defaults)
@@ -857,6 +857,23 @@
 	(progn (goto-char (match-beginning 1))
 	       (skip-chars-forward ";&|")))))
 
+(defun shell-pcomplete ()
+  "Proxy function which sets up comint for pcomplete.
+See `pcomplete'."
+  (interactive)
+  (pcomplete-comint-setup 'shell-dynamic-complete-functions)
+  (fset 'shell-pcomplete 'pcomplete)
+  (fset 'shell-pcomplete-reverse 'pcomplete-reverse)
+  (call-interactively #'pcomplete))
+
+(defun shell-pcomplete-reverse ()
+  "Proxy function which sets up comint for pcomplete.
+See `pcomplete'."
+  (interactive)
+  (pcomplete-comint-setup 'shell-dynamic-complete-functions)
+  (fset 'shell-pcomplete 'pcomplete)
+  (fset 'shell-pcomplete-reverse 'pcomplete-reverse)
+  (call-interactively #'pcomplete-reverse))
 
 (defun shell-dynamic-complete-command ()
   "Dynamically complete the command at point.

Re: using pcomplete for comint.el/shell.el

[Stefan Monnier]

> One of the design goals was to avoid loading pcomplete unless the user
> actually used the completion facilities.  So this made things a little
> bit tricky.  I've attached the patch to shell.el to implement this. 
> Other modes will have to do something very similar.  
> 
> Or would it be better if we just unconditionally required pcomplete.el
> for shell.el?
> 
> After this is implemented, we can just remove a lot of cruft in
> comint.el.

Sounds good.  On a related note, I really would like it if we could
make a `read-shell-command' that provides pcomplete completion for
things like M-! and friends.

> Also, one issue to consider is that this will break backwards
> compatibility with modes deriving from comint.

Could you expand on that ?  What kind of breakage ?
Can it be avoided ?


	Stefan

PS: I currently use the following quick-and-ugly hack for `shell-command':



--- simple.el	6 Feb 2002 15:08:45 -0000	1.520
+++ simple.el	15 Feb 2002 22:47:39 -0000
@@ -1135,6 +1152,20 @@
 is run interactively.  A value of nil means that output to stderr and
 stdout will be intermixed in the output stream.")
 
+(defun minibuffer-shell-complete ()
+  "Complete current minibuffer input as a shell command."
+  (interactive)
+  (require 'shell)
+  (pcomplete-shell-setup)
+  (let ((comint-dynamic-complete-functions shell-dynamic-complete-functions))
+    (call-interactively 'comint-dynamic-complete)))
+
+(defvar minibuffer-local-shell-completion-map
+  (let ((map (make-sparse-keymap)))
+    (set-keymap-parent map minibuffer-local-map)
+    (define-key map "\t" 'minibuffer-shell-complete)
+    map))
+
 (defun shell-command (command &optional output-buffer error-buffer)
   "Execute string COMMAND in inferior shell; display output, if any.
 With prefix argument, insert the COMMAND's output at point.
@@ -1186,7 +1217,7 @@
 specifies the value of ERROR-BUFFER."
 
   (interactive (list (read-from-minibuffer "Shell command: "
-					   nil nil nil 'shell-command-history)
+					   nil minibuffer-local-shell-completion-map nil 'shell-command-history)
 		     current-prefix-arg
 		     shell-command-default-error-buffer))
   ;; Look for a handler in case default-directory is a remote file name.
@@ -1399,8 +1430,9 @@
 		 ;; Do this before calling region-beginning
 		 ;; and region-end, in case subprocess output
 		 ;; relocates them while we are in the minibuffer.
-		 (setq string (read-from-minibuffer "Shell command on region: "
-						    nil nil nil
+		 (setq string (read-from-minibuffer
+			       "Shell command on region: "
+			       nil minibuffer-local-shell-completion-map nil
 						    'shell-command-history))
 		 ;; call-interactively recognizes region-beginning and
 		 ;; region-end specially, leaving them in the history.


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Colin Walters]

On Fri, 2002-02-15 at 17:49, Stefan Monnier wrote:

> Sounds good.  On a related note, I really would like it if we could
> make a `read-shell-command' that provides pcomplete completion for
> things like M-! and friends.

That would be nice.  It shouldn't be too hard.

> Could you expand on that ?  What kind of breakage ?
> Can it be avoided ?

Hm, I appear to have been on extra crack when I wrote that; it won't
break backwards compatibility at all.  

So, if there aren't any objections, I'll just go ahead and install this.


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Eli Zaretskii]

> From: "Stefan Monnier" <monnier+gnu/emacs@RUM.cs.yale.edu>
> Date: Fri, 15 Feb 2002 17:49:37 -0500
> 
> On a related note, I really would like it if we could
> make a `read-shell-command' that provides pcomplete completion for
> things like M-! and friends.

eshell-command has this, so perhaps we could lift some code from
there.

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Richard Stallman]

    +(defun minibuffer-shell-complete ()
    +  "Complete current minibuffer input as a shell command."
    +  (interactive)
    +  (require 'shell)
    +  (pcomplete-shell-setup)
    +  (let ((comint-dynamic-complete-functions shell-dynamic-complete-functions))
    +    (call-interactively 'comint-dynamic-complete)))

We might want to see if we could move comint-dynamic-complete and its
mechanisms into a separate file, so that using this command won't load
all of comint.

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Richard Stallman]

    +  "Proxy function which sets up comint for pcomplete.
    +See `pcomplete'."
    +  (interactive)
    +  (pcomplete-comint-setup 'shell-dynamic-complete-functions)
    +  (fset 'shell-pcomplete 'pcomplete)
    +  (fset 'shell-pcomplete-reverse 'pcomplete-reverse)
    +  (call-interactively #'pcomplete))

It should not redefine itself, that is confusing.  It should just call
pcomplete every time.

It needs a real doc string.


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Colin Walters]

[-- Attachment #1: Type: text/plain, Size: 319 bytes --]

On Sat, 2002-02-16 at 16:55, Richard Stallman wrote:

> It should not redefine itself, that is confusing.  It should just call
> pcomplete every time.

Hm.  I don't find it particularly confusing, but I guess it maybe does
try to be a bit too efficient at the expense of comprehensibility.  

How about this version?



[-- Attachment #2: shell.patch --]
[-- Type: text/plain, Size: 1979 bytes --]

--- shell.el.~1.105.~	Mon Dec  3 12:24:41 2001
+++ shell.el	Sat Feb 16 19:31:54 2002
@@ -310,7 +310,8 @@
        (setq shell-mode-map (nconc (make-sparse-keymap) comint-mode-map))
        (define-key shell-mode-map "\C-c\C-f" 'shell-forward-command)
        (define-key shell-mode-map "\C-c\C-b" 'shell-backward-command)
-       (define-key shell-mode-map "\t" 'comint-dynamic-complete)
+       (define-key shell-mode-map "\t" 'shell-pcomplete)
+       (define-key shell-mode-map "\M-\t" 'shell-pcomplete-reverse)
        (define-key shell-mode-map "\M-?"
 	 'comint-dynamic-list-filename-completions)
        (define-key shell-mode-map [menu-bar completion]
@@ -396,7 +397,6 @@
   (setq comint-delimiter-argument-list shell-delimiter-argument-list)
   (setq comint-file-name-chars shell-file-name-chars)
   (setq comint-file-name-quote-list shell-file-name-quote-list)
-  (setq comint-dynamic-complete-functions shell-dynamic-complete-functions)
   (make-local-variable 'paragraph-start)
   (setq paragraph-start comint-prompt-regexp)
   (make-local-variable 'font-lock-defaults)
@@ -857,6 +857,22 @@
 	(progn (goto-char (match-beginning 1))
 	       (skip-chars-forward ";&|")))))
 
+(defvar shell-pcomplete-setup-p nil)
+(defun shell-pcomplete ()
+  "Invoke `pcomplete', after ensuring this buffer is set up for it."
+  (interactive)
+  (unless (prog1 shell-pcomplete-setup-p
+	    (setq shell-pcomplete-setup-p t))
+    (pcomplete-comint-setup 'shell-dynamic-complete-functions))
+  (call-interactively #'pcomplete))
+
+(defun shell-pcomplete-reverse ()
+  "Invoke `pcomplete-reverse', after ensuring this buffer is set up for it."
+  (interactive)
+  (unless (prog1 shell-pcomplete-setup-p
+	    (setq shell-pcomplete-setup-p t))
+    (pcomplete-comint-setup 'shell-dynamic-complete-functions))
+  (call-interactively #'pcomplete-reverse))
 
 (defun shell-dynamic-complete-command ()
   "Dynamically complete the command at point.

Re: using pcomplete for comint.el/shell.el

[Richard Stallman]

This code is cleaner, but what about the doc string:

    +(defun shell-pcomplete ()
    +  "Invoke `pcomplete', after ensuring this buffer is set up for it."

The doc string should be self-contained.  It should say what this
command really does in a shell buffer, in concrete terms.

pcomplete-reverse is useful when cycling completion is in use.
Is cycling completion enabled in shell buffers?

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[John Wiegley]

>>>>> On Sun Feb 17, RMS writes:

> pcomplete-reverse is useful when cycling completion is in use.  Is
> cycling completion enabled in shell buffers?

By default, pcomplete uses cycling completion in all locations.  There
is a "cutoff", meaning if the list has more than five elements, then
it does not use cycling, but rather presents a buffer of options, like
standard the completion facilities.

John

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Colin Walters]

[-- Attachment #1: Type: text/plain, Size: 252 bytes --]

Well, it turns out that we can't really take the approach of creating a
proxy function for pcomplete in shell.el, because pcomplete looks at
last-command to determine its behavior.

So, is there any objection to just loading pcomplete with shell.el?



[-- Attachment #2: shell.patch --]
[-- Type: text/plain, Size: 1490 bytes --]

--- shell.el.~1.105.~	Mon Dec  3 12:24:41 2001
+++ shell.el	Mon Feb 18 16:47:59 2002
@@ -104,6 +104,7 @@
 ;;; Code:
 
 (require 'comint)
+(require 'pcomplete)
 
 ;;; Customization and Buffer Variables
 
@@ -310,7 +311,8 @@
        (setq shell-mode-map (nconc (make-sparse-keymap) comint-mode-map))
        (define-key shell-mode-map "\C-c\C-f" 'shell-forward-command)
        (define-key shell-mode-map "\C-c\C-b" 'shell-backward-command)
-       (define-key shell-mode-map "\t" 'comint-dynamic-complete)
+       (define-key shell-mode-map "\t" 'pcomplete)
+       (define-key shell-mode-map "\M-\t" 'pcomplete-reverse)
        (define-key shell-mode-map "\M-?"
 	 'comint-dynamic-list-filename-completions)
        (define-key shell-mode-map [menu-bar completion]
@@ -396,7 +398,6 @@
   (setq comint-delimiter-argument-list shell-delimiter-argument-list)
   (setq comint-file-name-chars shell-file-name-chars)
   (setq comint-file-name-quote-list shell-file-name-quote-list)
-  (setq comint-dynamic-complete-functions shell-dynamic-complete-functions)
   (make-local-variable 'paragraph-start)
   (setq paragraph-start comint-prompt-regexp)
   (make-local-variable 'font-lock-defaults)
@@ -856,7 +857,6 @@
 	 (format "[;&|]+[\t ]*\\(%s\\)" shell-command-regexp) limit 'move arg)
 	(progn (goto-char (match-beginning 1))
 	       (skip-chars-forward ";&|")))))
-
 
 (defun shell-dynamic-complete-command ()
   "Dynamically complete the command at point.

Re: using pcomplete for comint.el/shell.el

[John Wiegley]

>>>>> On Mon Feb 18, Colin writes:

> Well, it turns out that we can't really take the approach of
> creating a proxy function for pcomplete in shell.el, because
> pcomplete looks at last-command to determine its behavior.

If necessary, pcomplete could be changed to use a configurable list of
function symbols that it should test against, in order to determine
whether cycling should apply when the next TAB is hit...

> So, is there any objection to just loading pcomplete with shell.el?

Your patch does not call `pcomplete-shell-setup'.

John

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Richard Stallman]

    So, is there any objection to just loading pcomplete with shell.el?

I object because it is wasteful.

    Well, it turns out that we can't really take the approach of creating a
    proxy function for pcomplete in shell.el, because pcomplete looks at
    last-command to determine its behavior.

So make them set last-command.  It should be easy.


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel

Re: using pcomplete for comint.el/shell.el

[Colin Walters]

On Tue, 2002-02-19 at 16:31, Richard Stallman wrote:
> So make them set last-command.  It should be easy.

Ok, I've gone ahead and committed code which works this way.


_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://mail.gnu.org/mailman/listinfo/emacs-devel