From 9fcb2389cc9828be59c350bf02b578855c867ad3 Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Mon, 3 Jan 2022 17:28:00 -0800 Subject: [PATCH] Follow POSIX/GNU argument conventions for 'eshell-eval-using-options' * lisp/eshell/esh-opt.el (eshell--split-switch): New function. (eshell-set-option): Allow setting a supplied value instead of always consuming from 'eshell--args'. (eshell--process-option): Support consuming option values specified as a single token. (eshell--process-args): For short options, pass full switch token to 'eshell--process-option'. * test/lisp/eshell/esh-opt-tests.el (esh-opt-process-args-test): Fix test. (test-eshell-eval-using-options): Add tests for various types of options. * etc/NEWS: Announce the change. --- etc/NEWS | 3 + lisp/eshell/esh-opt.el | 90 +++++++++++++------- test/lisp/eshell/esh-opt-tests.el | 131 ++++++++++++++++++++++-------- 3 files changed, 158 insertions(+), 66 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index ca6a716ccd..b595d99633 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1060,6 +1060,9 @@ dimensions. Specifying a cons as the from argument allows to start measuring text from a specified amount of pixels above or below a position. +--- +** 'eshell-eval-using-options' now follows POSIX/GNU argument syntax conventions. + ** XDG support *** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable. diff --git a/lisp/eshell/esh-opt.el b/lisp/eshell/esh-opt.el index 7d31845528..fcc35780e9 100644 --- a/lisp/eshell/esh-opt.el +++ b/lisp/eshell/esh-opt.el @@ -187,49 +187,82 @@ eshell-show-usage will be called instead." extcmd))))) (throw 'eshell-usage usage))) -(defun eshell--set-option (name ai opt options opt-vals) +(defun eshell--split-switch (switch kind) + "Split SWITCH into its option name and potential value, if any. +KIND should be the integer 0 if SWITCH is a short option, or 1 if it's +a long option." + (if (eq kind 0) + ;; Short option + (cons (aref switch 0) + (and (> (length switch) 1) (substring switch 1))) + ;; Long option + (save-match-data + (string-match "\\([^=]*\\)\\(?:=\\(.*\\)\\)?" switch) + (cons (match-string 1 switch) (match-string 2 switch))))) + +(defun eshell--set-option (name ai opt value options opt-vals) "Using NAME's remaining args (index AI), set the OPT within OPTIONS. -If the option consumes an argument for its value, the argument list -will be modified." +VALUE is the potential value of the OPT, coming from args like +\"-fVALUE\" or \"--foo=VALUE\", or nil if no value was supplied. If +OPT doesn't consume a value, return VALUE unchanged so that it can be +processed later; otherwsie, return nil. + +If the OPT consumes an argument for its value and VALUE is nil, the +argument list will be modified." (if (not (nth 3 opt)) (eshell-show-usage name options) - (setcdr (assq (nth 3 opt) opt-vals) - (if (eq (nth 2 opt) t) - (if (> ai (length eshell--args)) - (error "%s: missing option argument" name) - (pop (nthcdr ai eshell--args))) - (or (nth 2 opt) t))))) + (if (eq (nth 2 opt) t) + (progn + (setcdr (assq (nth 3 opt) opt-vals) + (or value + (if (> ai (length eshell--args)) + (error "%s: missing option argument" name) + (pop (nthcdr ai eshell--args))))) + nil) + (setcdr (assq (nth 3 opt) opt-vals) + (or (nth 2 opt) t)) + value))) (defun eshell--process-option (name switch kind ai options opt-vals) "For NAME, process SWITCH (of type KIND), from args at index AI. The SWITCH will be looked up in the set of OPTIONS. -SWITCH should be either a string or character. KIND should be the -integer 0 if it's a character, or 1 if it's a string. - -The SWITCH is then be matched against OPTIONS. If no matching handler -is found, and an :external command is defined (and available), it will -be called; otherwise, an error will be triggered to say that the -switch is unrecognized." - (let* ((opts options) - found) +SWITCH should be a string starting with the option to process, +possibly followed by its value, e.g. \"u\" or \"uUSER\". KIND should +be the integer 0 if it's a short option, or 1 if it's a long option. + +The SWITCH is then be matched against OPTIONS. If KIND is 0 and the +SWITCH matches an option that doesn't take a value, return the +remaining characters in SWITCH to be processed later as further short +options. + +If no matching handler is found, and an :external command is defined +(and available), it will be called; otherwise, an error will be +triggered to say that the switch is unrecognized." + (let ((switch (eshell--split-switch switch kind)) + (opts options) + found remaining) (while opts (if (and (listp (car opts)) - (nth kind (car opts)) - (equal switch (nth kind (car opts)))) + (equal (car switch) (nth kind (car opts)))) (progn - (eshell--set-option name ai (car opts) options opt-vals) + (setq remaining (eshell--set-option name ai (car opts) + (cdr switch) options opt-vals)) + (when (and remaining (eq kind 1)) + (error "%s: option --%s doesn't allow an argument" + name (car switch))) (setq found t opts nil)) (setq opts (cdr opts)))) - (unless found + (if found + 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 switch) "%s: unrecognized option -%c" + (error (if (characterp (car switch)) "%s: unrecognized option -%c" "%s: unrecognized option --%s") - name switch))))))) + name (car switch)))))))) (defun eshell--process-args (name args options) "Process the given ARGS using OPTIONS." @@ -262,12 +295,9 @@ eshell--process-args (if (> (length switch) 0) (eshell--process-option name switch 1 ai options opt-vals) (setq ai (length eshell--args))) - (let ((len (length switch)) - (index 0)) - (while (< index len) - (eshell--process-option name (aref switch index) - 0 ai options opt-vals) - (setq index (1+ index)))))))) + (while (> (length switch) 0) + (setq switch (eshell--process-option name switch 0 + ai options opt-vals))))))) (nconc (mapcar #'cdr opt-vals) eshell--args))) (provide 'esh-opt) diff --git a/test/lisp/eshell/esh-opt-tests.el b/test/lisp/eshell/esh-opt-tests.el index e2a0ea59d1..f9203bd92a 100644 --- a/test/lisp/eshell/esh-opt-tests.el +++ b/test/lisp/eshell/esh-opt-tests.el @@ -57,7 +57,7 @@ esh-opt-process-args-test '((?u "user" t user "execute a command as another USER") :parse-leading-options-only)))) (should - (equal '("world" "emerge") + (equal '("DN" "emerge" "world") (eshell--process-args "sudo" '("-u" "root" "emerge" "-uDN" "world") @@ -65,59 +65,118 @@ esh-opt-process-args-test (ert-deftest test-eshell-eval-using-options () "Tests for `eshell-eval-using-options'." + ;; Test short options. (eshell-eval-using-options - "sudo" '("-u" "root" "whoami") - '((?u "user" t user "execute a command as another USER") - :parse-leading-options-only) - (should (equal user "root"))) + "ls" '("-a" "/dev/null") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all t)) + (should (equal args '("/dev/null")))) (eshell-eval-using-options - "sudo" '("--user" "root" "whoami") - '((?u "user" t user "execute a command as another USER") - :parse-leading-options-only) - (should (equal user "root"))) + "ls" '("/dev/null") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("/dev/null")))) + ;; Test long options. (eshell-eval-using-options - "sudo" '("emerge" "-uDN" "world") - '((?u "user" t user "execute a command as another USER")) - (should (equal user "world"))) + "ls" '("--all" "/dev/null") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all t)) + (should (equal args '("/dev/null")))) + + ;; Test options with constant values. (eshell-eval-using-options - "sudo" '("emerge" "-uDN" "world") - '((?u "user" t user "execute a command as another USER") - :parse-leading-options-only) - (should (eq user nil))) + "ls" '("/dev/null" "-h") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024)) + (should (equal args '("/dev/null")))) + (eshell-eval-using-options + "ls" '("/dev/null" "--human-readable") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024)) + (should (equal args '("/dev/null")))) + (eshell-eval-using-options + "ls" '("/dev/null") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eq human-readable nil)) + (should (equal args '("/dev/null")))) + ;; Test options with user-specified values. (eshell-eval-using-options "ls" '("-I" "*.txt" "/dev/null") '((?I "ignore" t ignore-pattern "do not list implied entries matching pattern")) - (should (equal ignore-pattern "*.txt"))) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/dev/null")))) + (eshell-eval-using-options + "ls" '("-I*.txt" "/dev/null") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/dev/null")))) + (eshell-eval-using-options + "ls" '("--ignore" "*.txt" "/dev/null") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/dev/null")))) + (eshell-eval-using-options + "ls" '("--ignore=*.txt" "/dev/null") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/dev/null")))) + ;; Test multiple short options in a single token. (eshell-eval-using-options - "ls" '("-l" "/dev/null") - '((?l nil long-listing listing-style + "ls" '("-al" "/dev/null") + '((?a "all" nil show-all + "do not ignore entries starting with .") + (?l nil long-listing listing-style "use a long listing format")) - (should (eql listing-style 'long-listing))) + (should (eq t show-all)) + (should (eql listing-style 'long-listing)) + (should (equal args '("/dev/null")))) (eshell-eval-using-options - "ls" '("/dev/null") - '((?l nil long-listing listing-style - "use a long listing format")) - (should (eq listing-style nil))) + "ls" '("-aI*.txt" "/dev/null") + '((?a "all" nil show-all + "do not ignore entries starting with .") + (?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (eq t show-all)) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/dev/null")))) + ;; Test :parse-leading-options-only. (eshell-eval-using-options - "ls" '("/dev/null" "-h") - '((?h "human-readable" 1024 human-readable - "print sizes in human readable format")) - (should (eql human-readable 1024))) + "sudo" '("-u" "root" "whoami") + '((?u "user" t user "execute a command as another USER") + :parse-leading-options-only) + (should (equal user "root")) + (should (equal args '("whoami")))) (eshell-eval-using-options - "ls" '("/dev/null" "--human-readable") - '((?h "human-readable" 1024 human-readable - "print sizes in human readable format")) - (should (eql human-readable 1024))) + "sudo" '("--user" "root" "whoami") + '((?u "user" t user "execute a command as another USER") + :parse-leading-options-only) + (should (equal user "root")) + (should (equal args '("whoami")))) (eshell-eval-using-options - "ls" '("/dev/null") - '((?h "human-readable" 1024 human-readable - "print sizes in human readable format")) - (should (eq human-readable nil)))) + "sudo" '("emerge" "-uDN" "world") + '((?u "user" t user "execute a command as another USER")) + (should (equal user "DN")) + (should (equal args '("emerge" "world")))) + (eshell-eval-using-options + "sudo" '("emerge" "-uDN" "world") + '((?u "user" t user "execute a command as another USER") + :parse-leading-options-only) + (should (eq user nil)) + (should (equal args '("emerge" "-uDN" "world"))))) (provide 'esh-opt-tests) -- 2.25.1