master 90de226e21 1/2: Raise an error from 'eval-eval-using-options' for unknown options

[Lars Ingebrigtsen]

branch: master
commit 90de226e218883f2a62ce3e1ba9a79ef8e1be70c
Author: Jim Porter <jporterbugs@gmail.com>
Commit: Lars Ingebrigtsen <larsi@gnus.org>

    Raise an error from 'eval-eval-using-options' for unknown options
    
    * lisp/eshell/em-basic.el (eshell/echo): Add -E option.
    
    * lisp/eshell/esh-opt.el (eshell--process-option): Raise an error if
    an unknown option is encountered, even when :external is nil.
    
    * test/lisp/eshell/esh-opt-tests.el (esh-opt-process-args-test)
    (test-eshell-eval-using-options): Add test cases for this.
---
 lisp/eshell/em-basic.el           | 13 ++++--
 lisp/eshell/esh-opt.el            | 12 +++---
 test/lisp/eshell/esh-opt-tests.el | 86 +++++++++++++++++++++++++++++----------
 3 files changed, 79 insertions(+), 32 deletions(-)

diff --git a/lisp/eshell/em-basic.el b/lisp/eshell/em-basic.el
index d3b15c900b..ba868cee59 100644
--- a/lisp/eshell/em-basic.el
+++ b/lisp/eshell/em-basic.el
@@ -113,11 +113,16 @@ or `eshell-printn' for display."
   "Implementation of `echo'.  See `eshell-plain-echo-behavior'."
   (eshell-eval-using-options
    "echo" args
-   '((?n nil (nil) output-newline "do not output the trailing newline")
-     (?N nil (t)   output-newline "terminate with a newline")
-     (?h "help" nil nil "output this help screen")
+   '((?n nil (nil) output-newline
+         "do not output the trailing newline")
+     (?N nil (t)   output-newline
+         "terminate with a newline")
+     (?E nil nil   _disable-escapes
+         "don't interpret backslash escapes (default)")
+     (?h "help" nil nil
+         "output this help screen")
      :preserve-args
-     :usage "[-n | -N] [object]")
+     :usage "[OPTION]... [OBJECT]...")
    (if eshell-plain-echo-behavior
        (eshell-echo args (if output-newline (car output-newline) t))
      ;; In Emacs 28.1 and earlier, "-n" was used to add a newline to
diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el
index bba1c4ad25..c802bee3af 100644
--- a/lisp/eshell/esh-opt.el
+++ b/lisp/eshell/esh-opt.el
@@ -257,12 +257,12 @@ triggered to say that the switch is unrecognized."
         remaining
       (let ((extcmd (memq ':external options)))
        (when extcmd
-         (setq extcmd (eshell-search-path (cadr extcmd)))
-         (if extcmd
-             (throw 'eshell-ext-command extcmd)
-            (error (if (characterp (car switch)) "%s: unrecognized option -%c"
-                     "%s: unrecognized option --%s")
-                   name (car switch))))))))
+         (setq extcmd (eshell-search-path (cadr extcmd))))
+       (if extcmd
+           (throw 'eshell-ext-command extcmd)
+          (error (if (characterp (car switch)) "%s: unrecognized option -%c"
+                   "%s: unrecognized option --%s")
+                 name (car switch)))))))
 
 (defun eshell--process-args (name args options)
   "Process the given ARGS using OPTIONS."
diff --git a/test/lisp/eshell/esh-opt-tests.el 
b/test/lisp/eshell/esh-opt-tests.el
index 255768635b..b76ed8866d 100644
--- a/test/lisp/eshell/esh-opt-tests.el
+++ b/test/lisp/eshell/esh-opt-tests.el
@@ -27,41 +27,63 @@
   (should
    (equal '(t)
           (eshell--process-args
-           "sudo"
-           '("-a")
-           '((?a "all" nil show-all "")))))
-  (should
-   (equal '(nil)
-          (eshell--process-args
-           "sudo"
-           '("-g")
-           '((?a "all" nil show-all "")))))
+           "sudo" '("-a")
+           '((?a "all" nil show-all
+                 "do not ignore entries starting with .")))))
   (should
    (equal '("root" "world")
           (eshell--process-args
-           "sudo"
-           '("-u" "root" "world")
-           '((?u "user" t user "execute a command as another USER")))))
+           "sudo" '("-u" "root" "world")
+           '((?u "user" t user
+                 "execute a command as another USER")))))
   (should
    (equal '(nil "emerge" "-uDN" "world")
           (eshell--process-args
-           "sudo"
-           '("emerge" "-uDN" "world")
-           '((?u "user" t user "execute a command as another USER")
+           "sudo" '("emerge" "-uDN" "world")
+           '((?u "user" t user
+                 "execute a command as another USER")
              :parse-leading-options-only))))
   (should
    (equal '("root" "emerge" "-uDN" "world")
           (eshell--process-args
-           "sudo"
-           '("-u" "root" "emerge" "-uDN" "world")
-           '((?u "user" t user "execute a command as another USER")
+           "sudo" '("-u" "root" "emerge" "-uDN" "world")
+           '((?u "user" t user
+                 "execute a command as another USER")
              :parse-leading-options-only))))
   (should
    (equal '("DN" "emerge" "world")
           (eshell--process-args
-           "sudo"
-           '("-u" "root" "emerge" "-uDN" "world")
-           '((?u "user" t user "execute a command as another USER"))))))
+           "sudo" '("-u" "root" "emerge" "-uDN" "world")
+           '((?u "user" t user
+                 "execute a command as another USER")))))
+
+  ;; Test :external.
+  (cl-letf (((symbol-function 'eshell-search-path) #'ignore))
+    (should
+     (equal '(nil "/some/path")
+            (eshell--process-args
+             "ls" '("/some/path")
+             '((?a "all" nil show-all
+                   "do not ignore entries starting with .")
+               :external "ls")))))
+  (cl-letf (((symbol-function 'eshell-search-path) #'identity))
+    (should
+     (equal '(no-catch eshell-ext-command "ls")
+            (should-error
+             (eshell--process-args
+              "ls" '("-u" "/some/path")
+              '((?a "all" nil show-all
+                    "do not ignore entries starting with .")
+                :external "ls"))
+             :type 'no-catch))))
+  (cl-letf (((symbol-function 'eshell-search-path) #'ignore))
+    (should-error
+     (eshell--process-args
+      "ls" '("-u" "/some/path")
+      '((?a "all" nil show-all
+            "do not ignore entries starting with .")
+        :external "ls"))
+     :type 'error)))
 
 (ert-deftest test-eshell-eval-using-options ()
   "Tests for `eshell-eval-using-options'."
@@ -190,7 +212,27 @@
    '((?u "user" t user "execute a command as another USER")
      :parse-leading-options-only)
    (should (eq user nil))
-   (should (equal args '("emerge" "-uDN" "world")))))
+   (should (equal args '("emerge" "-uDN" "world"))))
+
+  ;; Test unrecognized options.
+  (should-error
+   (eshell-eval-using-options
+    "ls" '("-u" "/some/path")
+    '((?a "all" nil show-all
+          "do not ignore entries starting with ."))
+    (ignore show-all)))
+  (should-error
+   (eshell-eval-using-options
+    "ls" '("-au" "/some/path")
+    '((?a "all" nil show-all
+          "do not ignore entries starting with ."))
+    (ignore show-all)))
+  (should-error
+   (eshell-eval-using-options
+    "ls" '("--unrecognized" "/some/path")
+    '((?a "all" nil show-all
+          "do not ignore entries starting with ."))
+    (ignore show-all))))
 
 (provide 'esh-opt-tests)