From e826440dc1173fc500c30225516b10c2639ce646 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 24 Feb 2023 21:49:54 -0800
Subject: [PATCH] Be more cautious in completing Eshell variable assignments

Previously, Eshell treated cases like the second argument in "tar
--directory=dir" as a variable assignment, but that prevented
'pcomplete/tar' from implementing its own completion for that
argument.

* lisp/eshell/esh-var.el (eshell-complete-variable-assignment): Only
handle completion when all initial arguments are variable assignments.

* test/lisp/eshell/em-cmpl-tests.el
(em-cmpl-test/variable-assign-completion/non-assignment): New test.
---
 lisp/eshell/esh-var.el            | 18 +++++++++++++-----
 test/lisp/eshell/em-cmpl-tests.el | 14 ++++++++++++++
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 0031324b537..5d6299af564 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -862,11 +862,19 @@ eshell-variables-list
 
 (defun eshell-complete-variable-assignment ()
   "If there is a variable assignment, allow completion of entries."
-  (let ((arg (pcomplete-actual-arg)) pos)
-    (when (string-match (concat "\\`" eshell-variable-name-regexp "=") arg)
-      (setq pos (match-end 0))
-      (if (string-match "\\(:\\)[^:]*\\'" arg)
-	  (setq pos (match-end 1)))
+  (catch 'not-assignment
+    ;; The current argument can only be a variable assignment if all
+    ;; arguments leading up to it are also variable assignments.  See
+    ;; `eshell-handle-local-variables'.
+    (dotimes (offset (1+ pcomplete-index))
+      (unless (string-match (concat "\\`" eshell-variable-name-regexp "=")
+                            (pcomplete-actual-arg 'first offset))
+        (throw 'not-assignment nil)))
+    ;; We have a variable assignment.  Handle it.
+    (let ((arg (pcomplete-actual-arg))
+          (pos (match-end 0)))
+      (when (string-match "\\(:\\)[^:]*\\'" arg)
+	(setq pos (match-end 1)))
       (setq pcomplete-stub (substring arg pos))
       (throw 'pcomplete-completions (pcomplete-entries)))))
 
diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el
index ecab7332822..be2199c0464 100644
--- a/test/lisp/eshell/em-cmpl-tests.el
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -217,6 +217,20 @@ em-cmpl-test/variable-assign-completion
      (should (equal (eshell-insert-and-complete "VAR=f")
                     "VAR=file.txt ")))))
 
+(ert-deftest em-cmpl-test/variable-assign-completion/non-assignment ()
+  "Test completion of things that look like variable assignment, but aren't.
+For example, the second argument in \"tar --directory=dir\" looks
+like it could be a variable assignment, but it's not.  We should
+let `pcomplete-tar' handle it instead.
+
+See <lisp/eshell/esh-var.el>."
+  (with-temp-eshell
+   (ert-with-temp-directory default-directory
+     (write-region nil nil (expand-file-name "file.txt"))
+     (make-directory "dir")
+     (should (equal (eshell-insert-and-complete "tar --directory=")
+                    "tar --directory=dir/")))))
+
 (ert-deftest em-cmpl-test/user-ref-completion ()
   "Test completion of user references like \"~user\".
 See <lisp/eshell/em-dirs.el>."
-- 
2.25.1