emacs-elpa-diffs
[Top][All Lists]
Advanced

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

[nongnu] elpa/bash-completion 6a53b5ca82 1/3: Fail fast when completion


From: ELPA Syncer
Subject: [nongnu] elpa/bash-completion 6a53b5ca82 1/3: Fail fast when completion is run against a process that is not bash.
Date: Sat, 4 Feb 2023 09:58:17 -0500 (EST)

branch: elpa/bash-completion
commit 6a53b5ca827faae378e0049c135adc3ccdb0df3e
Author: Stephane Zermatten <szermatt@gmx.net>
Commit: Stephane Zermatten <szermatt@gmx.net>

    Fail fast when completion is run against a process that is not bash.
    
    Before this change when the currently active command-line tool didn't
    answer as expected, bash-completion.el would wait for
    bash-completion-command-timeout for a response.
    bash-completion-command-timeout needs to be large, as some completion
    take a long time to execute.
    
    With this change, Bash sends a special string back before executing
    completion. Waiting for that string can be short, as it's cheap and fast
    for Bash to execute.
    
    Worst case, if the command-line tool isn't Bash anymore, the user now
    waits up to 600ms for an answer from Bash before getting back an error
    message. That new short timeout can be configured by customizing the
    value of bash-completion-short-command-timeout.
---
 bash-completion.el                       | 24 ++++++++++++++++++++++++
 test/bash-completion-integration-test.el | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/bash-completion.el b/bash-completion.el
index 755f37b500..77510fa81f 100644
--- a/bash-completion.el
+++ b/bash-completion.el
@@ -191,6 +191,20 @@ ignored."
   :type '(float)
   :group 'bash-completion)
 
+(defcustom bash-completion-short-command-timeout 0.6
+  "Number of seconds to wait for bash to start completion.
+
+This is the time it might take for Emacs to notice it's not
+actually talking to a functioning Bash process, when
+`bash-completion-use-separate-processes` is nil.
+
+This doesn't include the time it takes to execute completion,
+which can be quite long, but just the time it normally takes for
+the Bash process to respond to Emacs. This should be very short,
+unless the remote connection to the Bash process is very slow."
+  :type '(float)
+  :group 'bash-completion)
+
 (defcustom bash-completion-command-timeout 30
   "Number of seconds to wait for an answer from programmable completion 
functions.
 
@@ -1425,6 +1439,7 @@ and would like bash completion in Emacs to take these 
changes into account."
 
 (defun bash-completion--wait-for-regexp (process prompt-regexp timeout 
&optional limit)
   (let ((no-timeout t))
+    (goto-char (point-max))
     (while (and no-timeout
                 (not (re-search-backward prompt-regexp limit t)))
       (setq no-timeout (accept-process-output process timeout nil t)))
@@ -1465,6 +1480,7 @@ Return the status code of the command, as a number."
           (unless bash-completion-use-separate-processes
             (concat
              "set +o emacs; set +o vi;"
+             "echo \"==emacs==bash=${BASH_VERSINFO[0]}==.\";"
              "if [[ -z \"${__emacs_complete_ps1}\" ]]; then "
              " __emacs_complete_ps1=\"$PS1\";"
              " __emacs_complete_pc=\"$PROMPT_COMMAND\";"
@@ -1497,6 +1513,14 @@ Return the status code of the command, as a number."
     (with-current-buffer (bash-completion--get-buffer process)
       (erase-buffer)
       (funcall send-string process complete-command)
+      (unless bash-completion-use-separate-processes
+        (unless (bash-completion--wait-for-regexp
+                 process "==emacs==bash=[0-9]==." 
bash-completion-short-command-timeout)
+          (push (cons 'error "short-timeout") bash-completion--debug-info)
+          (push (cons 'buffer-string (buffer-substring-no-properties 
(point-min) (point-max)))
+                bash-completion--debug-info)
+          (error "Bash completion failed.  M-x bash-completion-debug for 
details"))
+        (delete-region (match-beginning 0) (1+ (match-end 0))))
       (unless (bash-completion--wait-for-regexp process 
"==emacs==ret=-?[[:digit:]]+==." timeout)
         (push (cons 'error "timeout") bash-completion--debug-info)
         (push (cons 'buffer-string (buffer-substring-no-properties (point-min) 
(point-max)))
diff --git a/test/bash-completion-integration-test.el 
b/test/bash-completion-integration-test.el
index 62b7ffac86..8fd72d2d54 100644
--- a/test/bash-completion-integration-test.el
+++ b/test/bash-completion-integration-test.el
@@ -33,6 +33,11 @@
 (require 'dired)
 (require 'ert)
 
+(defvar bash-completion_test-notbash-prog (executable-find "dash")
+  "Path to a command line executable that is not bash.
+
+It must exit when given C-d as input.")
+
 (defvar bash-completion_test-setup-completion "/etc/bash_completion")
 (defconst bash-completion_test-start-mark "====START====")
 
@@ -288,6 +293,33 @@ across Emacs version."
       ;; actually bash, the test could otherwise work just fine.
       (should (not (null (cdr (assq nil bash-completion-processes)))))))))
 
+(ert-deftest bash-completion-integration-notbash-anymore-test ()
+  (skip-unless bash-completion_test-notbash-prog)
+  (bash-completion_test-with-shell-harness
+   "" ; bashrc
+   nil ; use-separate-process
+   ;; initially completion works
+   (should (bash-completion_test-equal-any-order
+            '("some/directory/" "some/other/")
+            (bash-completion_test-candidates "ls some/")))
+   ;; but then later on bash executes another command-line tool
+   (delete-region (line-beginning-position) (line-end-position))
+   (insert bash-completion_test-notbash-prog)
+   (comint-send-input)
+   (let ((bash-completion--debug-info nil))
+     (should-error (bash-completion_test-candidates "ls some/"))
+     (should (equal "short-timeout" (cdr (assq 'error 
bash-completion--debug-info)))))
+
+   ;; this is recoverable, once the other command-line tool exits,
+   ;; completion works again.
+   (delete-region (line-beginning-position) (line-end-position))
+   (insert "\004") ;; C-d
+   (comint-send-input)
+
+   (should (bash-completion_test-equal-any-order
+            '("some/directory/" "some/other/")
+            (bash-completion_test-candidates "ls some/")))))
+
 (ert-deftest bash-completion-integration-space ()
   (bash-completion_test-with-shell-harness
    ""



reply via email to

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