* bug#52999: 29.0.50; [PATCH] `eshell-eval-using-options' should follow POSIX/GNU argument conventions @ 2022-01-04 1:36 Jim Porter 2022-01-04 5:33 ` bug#52999: 29.0.50; [PATCH v2] " Jim Porter 0 siblings, 1 reply; 10+ messages in thread From: Jim Porter @ 2022-01-04 1:36 UTC (permalink / raw) To: 52999 [-- Attachment #1: Type: text/plain, Size: 1975 bytes --] Currently, `eshell-eval-using-options' doesn't follow POSIX/GNU argument conventions[1], resulting in some confusing behavior. To see this in action, the easiest way is probably to make a small patch to `eshell-do-ls' in lisp/eshell/em-ls.el: just comment out the line that says, :external "ls" and then eval the function again. (This is necessary so that eshell doesn't fall back to the system's `ls' command when it gets confused.) Then open `eshell' and run: ls '-I*.el' Instead of what you'd expect (a directory listing that ignores Emacs Lisp files), instead you get a directory listing of *all* the files in the long listing format. That's because `eshell-eval-using-options' assumes that all the characters after the "-" are names of short options, rather than a single short option followed by its value. You can see a similar problem with: ls '--ignore=*.el' In this case, `eshell-eval-using-options' looks for an option named "ignore=*.el" instead of an option named "ignore" followed by its value. I've attached a patch with tests to fix this and use the POSIX/GNU argument conventions, supporting both the above cases. However, I should mention that this is a slightly incompatible change. A small number of existing eshell commands work like `ls -I', and their behavior is now a bit different. Previously, you could do the following, ls -Ia '*.el' to list all the files in a directory, excluding Emacs Lisp files. Now, you have to spell that as: ls -aI '*.el' # or... ls -aI'*.el' I think that's ok though, since I can't imagine anyone *wanting* the old behavior. It surprised me quite a bit when I stumbled across it, and worse, it only crops up sometimes, since eshell transparently falls back to the real commands when it gets confused. For completeness, the following commands+options are affected: sudo -u du -d ls -I [1] https://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html [-- Attachment #2: 0001-Follow-POSIX-GNU-argument-conventions-for-eshell-eva.patch --] [-- Type: text/plain, Size: 13091 bytes --] From 9fcb2389cc9828be59c350bf02b578855c867ad3 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v2] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-04 1:36 bug#52999: 29.0.50; [PATCH] `eshell-eval-using-options' should follow POSIX/GNU argument conventions Jim Porter @ 2022-01-04 5:33 ` Jim Porter 2022-01-04 13:01 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Jim Porter @ 2022-01-04 5:33 UTC (permalink / raw) To: 52999 [-- Attachment #1: Type: text/plain, Size: 331 bytes --] On 1/3/2022 5:36 PM, Jim Porter wrote: > I've attached a patch with tests to fix this and use the POSIX/GNU > argument conventions, supporting both the above cases. One small addition here. I just noticed that `eshell-eval-using-options' already supports "--" to terminate all the options, so I added a test case for that too. [-- Attachment #2: 0001-Follow-POSIX-GNU-argument-conventions-for-eshell-eva.patch --] [-- Type: text/plain, Size: 13705 bytes --] From de20c52f7669367479c3c4762cb8326f9a2058b3 Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Mon, 3 Jan 2022 21:30:59 -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 | 149 ++++++++++++++++++++++-------- 3 files changed, 174 insertions(+), 68 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..235e03bced 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,132 @@ 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"))) + "do not list implied entries matching pattern")) + (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 - "use a long listing format")) - (should (eql listing-style 'long-listing))) + "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 (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 that "--" terminates options. (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))) + "ls" '("--" "-a") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("-a")))) (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))) + "ls" '("--" "--all") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("--all")))) + + ;; Test :parse-leading-options-only. (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" '("-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 + "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 + "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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v2] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-04 5:33 ` bug#52999: 29.0.50; [PATCH v2] " Jim Porter @ 2022-01-04 13:01 ` Eli Zaretskii 2022-01-04 21:09 ` bug#52999: 29.0.50; [PATCH v3] " Jim Porter 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2022-01-04 13:01 UTC (permalink / raw) To: Jim Porter; +Cc: 52999 > From: Jim Porter <jporterbugs@gmail.com> > Date: Mon, 3 Jan 2022 21:33:28 -0800 > > --- 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. This is too terse: we cannot assume that everyone knows what does "POSIX/GNU argument syntax conventions" stand for. Especially since you didn't even say "command-line arguments", just "arguments". Please make the entry more informative. And why don't people who propose and install changes in Eshell also update the Eshell manual? That the manual in its current shape leaves a lot to be desired is not a justification to leave it that way. We will never improve that manual unless we start adding useful stuff to it, one small piece at a time. > (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")))) Can these tests be made less platform-specific? For example, not all the supported platforms have /dev/null, and we have a portable abstraction for it. > + "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 > + "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 > + "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"))))) And here, sudo and whoami don't necessarily exist, so something should be done about that, I think. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-04 13:01 ` Eli Zaretskii @ 2022-01-04 21:09 ` Jim Porter 2022-01-05 14:50 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Jim Porter @ 2022-01-04 21:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 52999 [-- Attachment #1: Type: text/plain, Size: 3730 bytes --] Thanks, I've attached an updated patch. On 1/4/2022 5:01 AM, Eli Zaretskii wrote: >> From: Jim Porter <jporterbugs@gmail.com> >> Date: Mon, 3 Jan 2022 21:33:28 -0800 >> >> --- 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. > > This is too terse: we cannot assume that everyone knows what does > "POSIX/GNU argument syntax conventions" stand for. Especially since > you didn't even say "command-line arguments", just "arguments". > Please make the entry more informative. Ok, I added a brief description explaining what specifically has been changed. > And why don't people who propose and install changes in Eshell also > update the Eshell manual? That the manual in its current shape leaves > a lot to be desired is not a justification to leave it that way. We > will never improve that manual unless we start adding useful stuff to > it, one small piece at a time. I just wasn't sure if `eshell-eval-using-options' should be in the manual or not. Thinking it over a bit more, it would have helped me if it had been in the manual (I encountered this bug while trying to write my own Eshell built-in command), so I added some info about it to the manual, mostly adapted from the docstring for `eshell-eval-using-options'. Hopefully I followed the right conventions here; I'm only vaguely familiar with the Texinfo format. >> (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")))) > > Can these tests be made less platform-specific? For example, not all > the supported platforms have /dev/null, and we have a portable > abstraction for it. They should actually work cross-platform, since the tests don't invoke the commands at all; they just make sure that `eshell-eval-using-options' can parse the switches correctly. To make this a bit clearer though, I replaced "/dev/null" with "/some/path". Hopefully when people see that, they'll understand that this is a "fake" path not corresponding to anything on the actual filesystem. >> + "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")))) [snip] > > And here, sudo and whoami don't necessarily exist, so something should > be done about that, I think. The same applies here; the commands aren't actually invoked, so they could just as easily be named "foo" and "bar". I think the reason for them looking like real commands is just so that a reader can glance at them and understand more-readily what the expected result is. Readers are likely to be familiar with "sudo" and "whoami", but wouldn't have any preconceptions about the semantics of (fake) commands named "foo" and "bar". If you still think it's a problem, I can change it though. (Also, technically, both of those commands should always exist in Eshell, since they're defined as built-in commands. "sudo" runs the TRAMP sudo method, and "whoami" is a TRAMP-aware Lisp implementation.) [-- Attachment #2: 0001-Follow-POSIX-GNU-argument-conventions-for-eshell-eva.patch --] [-- Type: text/plain, Size: 18573 bytes --] From 85a8e2df855f824c18adc70a4968bd3ae26f558d Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Tue, 4 Jan 2022 12:58:38 -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. * doc/misc/eshell.texi (Defining new built-in commands): New subsection, describe how to use 'eshell-eval-using-options'. * etc/NEWS: Announce the change. --- doc/misc/eshell.texi | 117 +++++++++++++++++++++++ etc/NEWS | 5 + lisp/eshell/esh-opt.el | 90 ++++++++++++------ test/lisp/eshell/esh-opt-tests.el | 151 ++++++++++++++++++++++-------- 4 files changed, 294 insertions(+), 69 deletions(-) diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi index a87dd4308c..1bc1fcbe9c 100644 --- a/doc/misc/eshell.texi +++ b/doc/misc/eshell.texi @@ -694,6 +694,123 @@ Built-ins supports Tramp. @end table +@subsection Defining new built-in commands +While Eshell can run Lisp functions directly as commands, it may be +more convenient to provide a special built-in command for +Eshell. Built-in commands are just ordinary Lisp functions designed +to be called from Eshell. When defining an Eshell-specific version of +an existing function, you can give that function a name starting with +@code{eshell/} so that Eshell knows to use it. + +@defmac eshell-eval-using-options name macro-args options body@dots{} +This macro processes a list of @var{macro-args} for the command +@var{name} using a set of command line @var{options}. If the +arguments are parsed successfully, it will store the resulting values +in local symbols and execute @var{body}; any remaining arguments will +be available in the locally let-bound variable @code{args}. The +return value is the value of the last form in @var{body}. + +If an unknown option was passed in @var{macro-args} and an external +command was specified (see below), this macro will start a process for +that command and throw the tag @code{eshell-external} with the new +process as its value. + +@var{options} should be a list beginning with one or more elements of +the following form, with each element representing a particular +command-line switch: + +@example +(@var{short} @var{long} @var{value} @var{symbol} @var{help-string}) +@end example + +@table @var +@item short +This element, if non-nil, should be a character to be used as a short +switch, like @code{-@var{short}}. At least one of this element and +@var{long} must be non-nil. + +@item long +This element, if non-nil, should be a string to be used as a long +switch, like @code{--@var{long}}. + +@item value +This element is the value associated with the option. It can be +either: + +@table @asis +@item @code{t} +The option needs a value to be specified after the switch. + +@item @code{nil} +The option is given the value @code{t}. + +@item anything else +The option is given the specified value. +@end table + +@item symbol +This element is the name of the Lisp symbol that will be bound to +@var{value}. If @var{symbol} is @code{nil}, specifying this switch +will instead call @code{eshell-show-usage}, and so is appropriate for +an option like @code{--help}. + +@item help-string +This element is a documentation string for the option, which will be +displayed when @code{eshell-show-usage} is invoked. +@end table + +After the list of command-line switch elements, @var{options} can +include additional keyword arguments to control how +@code{eshell-eval-using-options} behaves. Some of these take +arguments, while others don't. The recognized keywords are: + +@table @code +@item :external @var{string} +Specify @var{string} as an external command to run if there are +unknown switches in @var{macro-args}. + +@item :usage @var{string} +Set @var{string} as the initial part of the command's documentation +string. It appears before the options are listed. + +@item :post-usage @var{string} +Set @var{string} to be the (optional) trailing part of the command's +documentation string. It appears after the list of options, but +before the final part of the documentation about the associated +external command, if there is one. + +@item :show-usage +If present, then show the usage message if the command is called with +no arguments. + +@item :preserve-args +If present, do not pass @var{macro-args} through @code{flatten-tree} +and @code{eshell-stringify-list}. + +@item :parse-leading-options-only +If present, do not parse dash or switch arguments after the first +positional argument. Instead, treat them as positional arguments +themselves. +@end table + +For example, you could handle a subset of the options for the +@code{ls} command like this: + +@example +(eshell-eval-using-options + "ls" macro-args + '((?a nil nil show-all "show all files") + (?I "ignore" t ignore-pattern "ignore files matching pattern") + (nil "help" nil nil "show this help message") + :external "ls" + :usage "[OPTION]... [FILE]... + List information about FILEs (the current directory by default).") + ;; List the files in ARGS somehow... + ) +@end example + +@end defmac + @subsection Built-in variables Eshell knows a few built-in variables: diff --git a/etc/NEWS b/etc/NEWS index ca6a716ccd..cf392ca47e 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1060,6 +1060,11 @@ 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. +This now accepts command-line options with values passed as a single +token, such as '-oVALUE' or '--option=VALUE'. + ** 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..c13f5e15b3 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,132 @@ 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" "/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all t)) + (should (equal args '("/some/path")))) (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" '("/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("/some/path")))) + ;; 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" "/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all t)) + (should (equal args '("/some/path")))) + + ;; 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" '("/some/path" "-h") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024)) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("/some/path" "--human-readable") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024)) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("/some/path") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eq human-readable nil)) + (should (equal args '("/some/path")))) + ;; Test options with user-specified values. + (eshell-eval-using-options + "ls" '("-I" "*.txt" "/some/path") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("-I*.txt" "/some/path") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) (eshell-eval-using-options - "ls" '("-I" "*.txt" "/dev/null") + "ls" '("--ignore" "*.txt" "/some/path") '((?I "ignore" t ignore-pattern - "do not list implied entries matching pattern")) - (should (equal ignore-pattern "*.txt"))) + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("--ignore=*.txt" "/some/path") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) + ;; Test multiple short options in a single token. (eshell-eval-using-options - "ls" '("-l" "/dev/null") - '((?l nil long-listing listing-style - "use a long listing format")) - (should (eql listing-style 'long-listing))) + "ls" '("-al" "/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .") + (?l nil long-listing listing-style + "use a long listing format")) + (should (eq t show-all)) + (should (eql listing-style 'long-listing)) + (should (equal args '("/some/path")))) (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" "/some/path") + '((?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 '("/some/path")))) + ;; Test that "--" terminates options. (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))) + "ls" '("--" "-a") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("-a")))) (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))) + "ls" '("--" "--all") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("--all")))) + + ;; Test :parse-leading-options-only. (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" '("-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 + "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 + "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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-04 21:09 ` bug#52999: 29.0.50; [PATCH v3] " Jim Porter @ 2022-01-05 14:50 ` Eli Zaretskii 2022-01-06 0:48 ` Jim Porter 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2022-01-05 14:50 UTC (permalink / raw) To: Jim Porter; +Cc: 52999 > Cc: 52999@debbugs.gnu.org > From: Jim Porter <jporterbugs@gmail.com> > Date: Tue, 4 Jan 2022 13:09:29 -0800 > > I just wasn't sure if `eshell-eval-using-options' should be in the > manual or not. Thinking it over a bit more, it would have helped me if > it had been in the manual (I encountered this bug while trying to write > my own Eshell built-in command), so I added some info about it to the > manual, mostly adapted from the docstring for > `eshell-eval-using-options'. Hopefully I followed the right conventions > here; I'm only vaguely familiar with the Texinfo format. Yes, the documentation part is fine, modulo some minor comments below. > > Can these tests be made less platform-specific? For example, not all > > the supported platforms have /dev/null, and we have a portable > > abstraction for it. > > They should actually work cross-platform, since the tests don't invoke > the commands at all; they just make sure that > `eshell-eval-using-options' can parse the switches correctly. To make > this a bit clearer though, I replaced "/dev/null" with "/some/path". > Hopefully when people see that, they'll understand that this is a "fake" > path not corresponding to anything on the actual filesystem. Apologies for misreading this part of the code. > +@item symbol > +This element is the name of the Lisp symbol that will be bound to > +@var{value}. Is it a symbol or its name (a string)? You say "name", but the example: > If @var{symbol} is @code{nil}, specifying this switch uses a symbol, not its name. > +@item :preserve-args > +If present, do not pass @var{macro-args} through @code{flatten-tree} > +and @code{eshell-stringify-list}. I think this should explain the effect of that, or the difference between using and not using this keyword. > +--- > +** 'eshell-eval-using-options' now follows POSIX/GNU argument syntax conventions. > +This now accepts command-line options with values passed as a single ^^^^ "Eshell" instead of "This" will make it more clear what you mean. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-05 14:50 ` Eli Zaretskii @ 2022-01-06 0:48 ` Jim Porter 2022-01-06 12:31 ` Eli Zaretskii 2022-01-12 15:00 ` Eli Zaretskii 0 siblings, 2 replies; 10+ messages in thread From: Jim Porter @ 2022-01-06 0:48 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 52999 [-- Attachment #1: Type: text/plain, Size: 1423 bytes --] On 1/5/2022 6:50 AM, Eli Zaretskii wrote: >> Cc: 52999@debbugs.gnu.org >> From: Jim Porter <jporterbugs@gmail.com> >> Date: Tue, 4 Jan 2022 13:09:29 -0800 >> >> +@item symbol >> +This element is the name of the Lisp symbol that will be bound to >> +@var{value}. > > Is it a symbol or its name (a string)? You say "name", but the > example: > >> If @var{symbol} is @code{nil}, specifying this switch > > uses a symbol, not its name. Good catch. I've fixed this to say that it's the Lisp symbol. >> +@item :preserve-args >> +If present, do not pass @var{macro-args} through @code{flatten-tree} >> +and @code{eshell-stringify-list}. > > I think this should explain the effect of that, or the difference > between using and not using this keyword. I had to do a bit of digging to figure out what this keyword is supposed to do in practice. It seems that it's used when a built-in Eshell command wants to be able to accept arbitrary Lisp objects as arguments, instead of working with just a flat list of strings. I've added more detail to this paragraph. >> +--- >> +** 'eshell-eval-using-options' now follows POSIX/GNU argument syntax conventions. >> +This now accepts command-line options with values passed as a single > ^^^^ > "Eshell" instead of "This" will make it more clear what you mean. Ok, I updated this to refer to "Built-in commands in Eshell". Thanks for looking over the patch. [-- Attachment #2: 0001-Follow-POSIX-GNU-argument-conventions-for-eshell-eva.patch --] [-- Type: text/plain, Size: 18775 bytes --] From 45f48041c64915b4c05024b426c163bb419d083d Mon Sep 17 00:00:00 2001 From: Jim Porter <jporterbugs@gmail.com> Date: Tue, 4 Jan 2022 12:58:38 -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. * doc/misc/eshell.texi (Defining new built-in commands): New subsection, describe how to use 'eshell-eval-using-options'. * etc/NEWS: Announce the change. --- doc/misc/eshell.texi | 120 ++++++++++++++++++++++++ etc/NEWS | 6 ++ lisp/eshell/esh-opt.el | 90 ++++++++++++------ test/lisp/eshell/esh-opt-tests.el | 151 ++++++++++++++++++++++-------- 4 files changed, 298 insertions(+), 69 deletions(-) diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi index a87dd4308c..d72ca5afe9 100644 --- a/doc/misc/eshell.texi +++ b/doc/misc/eshell.texi @@ -694,6 +694,126 @@ Built-ins supports Tramp. @end table +@subsection Defining new built-in commands +While Eshell can run Lisp functions directly as commands, it may be +more convenient to provide a special built-in command for +Eshell. Built-in commands are just ordinary Lisp functions designed +to be called from Eshell. When defining an Eshell-specific version of +an existing function, you can give that function a name starting with +@code{eshell/} so that Eshell knows to use it. + +@defmac eshell-eval-using-options name macro-args options body@dots{} +This macro processes a list of @var{macro-args} for the command +@var{name} using a set of command line @var{options}. If the +arguments are parsed successfully, it will store the resulting values +in local symbols and execute @var{body}; any remaining arguments will +be available in the locally let-bound variable @code{args}. The +return value is the value of the last form in @var{body}. + +If an unknown option was passed in @var{macro-args} and an external +command was specified (see below), this macro will start a process for +that command and throw the tag @code{eshell-external} with the new +process as its value. + +@var{options} should be a list beginning with one or more elements of +the following form, with each element representing a particular +command-line switch: + +@example +(@var{short} @var{long} @var{value} @var{symbol} @var{help-string}) +@end example + +@table @var +@item short +This element, if non-nil, should be a character to be used as a short +switch, like @code{-@var{short}}. At least one of this element and +@var{long} must be non-nil. + +@item long +This element, if non-nil, should be a string to be used as a long +switch, like @code{--@var{long}}. + +@item value +This element is the value associated with the option. It can be +either: + +@table @asis +@item @code{t} +The option needs a value to be specified after the switch. + +@item @code{nil} +The option is given the value @code{t}. + +@item anything else +The option is given the specified value. +@end table + +@item symbol +This element is the Lisp symbol that will be bound to @var{value}. If +@var{symbol} is @code{nil}, specifying this switch will instead call +@code{eshell-show-usage}, and so is appropriate for an option like +@code{--help}. + +@item help-string +This element is a documentation string for the option, which will be +displayed when @code{eshell-show-usage} is invoked. +@end table + +After the list of command-line switch elements, @var{options} can +include additional keyword arguments to control how +@code{eshell-eval-using-options} behaves. Some of these take +arguments, while others don't. The recognized keywords are: + +@table @code +@item :external @var{string} +Specify @var{string} as an external command to run if there are +unknown switches in @var{macro-args}. + +@item :usage @var{string} +Set @var{string} as the initial part of the command's documentation +string. It appears before the options are listed. + +@item :post-usage @var{string} +Set @var{string} to be the (optional) trailing part of the command's +documentation string. It appears after the list of options, but +before the final part of the documentation about the associated +external command, if there is one. + +@item :show-usage +If present, then show the usage message if the command is called with +no arguments. + +@item :preserve-args +Normally, @code{eshell-eval-using-options} flattens the list of +arguments in @var{macro-args} and converts each to a string. If this +keyword is present, avoid doing that, instead preserving the original +arguments. This is useful for commands which want to accept arbitrary +Lisp objects. + +@item :parse-leading-options-only +If present, do not parse dash or switch arguments after the first +positional argument. Instead, treat them as positional arguments +themselves. +@end table + +For example, you could handle a subset of the options for the +@code{ls} command like this: + +@example +(eshell-eval-using-options + "ls" macro-args + '((?a nil nil show-all "show all files") + (?I "ignore" t ignore-pattern "ignore files matching pattern") + (nil "help" nil nil "show this help message") + :external "ls" + :usage "[OPTION]... [FILE]... + List information about FILEs (the current directory by default).") + ;; List the files in ARGS somehow... + ) +@end example + +@end defmac + @subsection Built-in variables Eshell knows a few built-in variables: diff --git a/etc/NEWS b/etc/NEWS index ca6a716ccd..e6bcbf5a86 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -1060,6 +1060,12 @@ 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. +Built-in commands in Eshell now accept command-line options with +values passed as a single token, such as '-oVALUE' or +'--option=VALUE'. + ** 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..c13f5e15b3 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,132 @@ 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" "/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all t)) + (should (equal args '("/some/path")))) (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" '("/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("/some/path")))) + ;; 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" "/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all t)) + (should (equal args '("/some/path")))) + + ;; 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" '("/some/path" "-h") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024)) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("/some/path" "--human-readable") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eql human-readable 1024)) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("/some/path") + '((?h "human-readable" 1024 human-readable + "print sizes in human readable format")) + (should (eq human-readable nil)) + (should (equal args '("/some/path")))) + ;; Test options with user-specified values. + (eshell-eval-using-options + "ls" '("-I" "*.txt" "/some/path") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("-I*.txt" "/some/path") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) (eshell-eval-using-options - "ls" '("-I" "*.txt" "/dev/null") + "ls" '("--ignore" "*.txt" "/some/path") '((?I "ignore" t ignore-pattern - "do not list implied entries matching pattern")) - (should (equal ignore-pattern "*.txt"))) + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) + (eshell-eval-using-options + "ls" '("--ignore=*.txt" "/some/path") + '((?I "ignore" t ignore-pattern + "do not list implied entries matching pattern")) + (should (equal ignore-pattern "*.txt")) + (should (equal args '("/some/path")))) + ;; Test multiple short options in a single token. (eshell-eval-using-options - "ls" '("-l" "/dev/null") - '((?l nil long-listing listing-style - "use a long listing format")) - (should (eql listing-style 'long-listing))) + "ls" '("-al" "/some/path") + '((?a "all" nil show-all + "do not ignore entries starting with .") + (?l nil long-listing listing-style + "use a long listing format")) + (should (eq t show-all)) + (should (eql listing-style 'long-listing)) + (should (equal args '("/some/path")))) (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" "/some/path") + '((?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 '("/some/path")))) + ;; Test that "--" terminates options. (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))) + "ls" '("--" "-a") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("-a")))) (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))) + "ls" '("--" "--all") + '((?a "all" nil show-all + "do not ignore entries starting with .")) + (should (eq show-all nil)) + (should (equal args '("--all")))) + + ;; Test :parse-leading-options-only. (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" '("-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 + "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 + "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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-06 0:48 ` Jim Porter @ 2022-01-06 12:31 ` Eli Zaretskii 2022-01-08 19:58 ` Jim Porter 2022-01-12 15:00 ` Eli Zaretskii 1 sibling, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2022-01-06 12:31 UTC (permalink / raw) To: Jim Porter; +Cc: 52999 > Cc: 52999@debbugs.gnu.org > From: Jim Porter <jporterbugs@gmail.com> > Date: Wed, 5 Jan 2022 16:48:39 -0800 > > Thanks for looking over the patch. Thanks, the new patch LGTM. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-06 12:31 ` Eli Zaretskii @ 2022-01-08 19:58 ` Jim Porter 2022-01-08 20:10 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Jim Porter @ 2022-01-08 19:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 52999 On 1/6/2022 4:31 AM, Eli Zaretskii wrote: >> Cc: 52999@debbugs.gnu.org >> From: Jim Porter <jporterbugs@gmail.com> >> Date: Wed, 5 Jan 2022 16:48:39 -0800 >> >> Thanks for looking over the patch. > > Thanks, the new patch LGTM. Ok, I think this should be ready to merge then? (I don't have commit access, so I can't merge it myself.) I have a couple other Eshell improvements that I'm working on, but I'll file separate bugs for those. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-08 19:58 ` Jim Porter @ 2022-01-08 20:10 ` Eli Zaretskii 0 siblings, 0 replies; 10+ messages in thread From: Eli Zaretskii @ 2022-01-08 20:10 UTC (permalink / raw) To: Jim Porter; +Cc: 52999 > Cc: 52999@debbugs.gnu.org > From: Jim Porter <jporterbugs@gmail.com> > Date: Sat, 8 Jan 2022 11:58:29 -0800 > > On 1/6/2022 4:31 AM, Eli Zaretskii wrote: > >> Cc: 52999@debbugs.gnu.org > >> From: Jim Porter <jporterbugs@gmail.com> > >> Date: Wed, 5 Jan 2022 16:48:39 -0800 > >> > >> Thanks for looking over the patch. > > > > Thanks, the new patch LGTM. > > Ok, I think this should be ready to merge then? If no further comments are posted, yes. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions 2022-01-06 0:48 ` Jim Porter 2022-01-06 12:31 ` Eli Zaretskii @ 2022-01-12 15:00 ` Eli Zaretskii 1 sibling, 0 replies; 10+ messages in thread From: Eli Zaretskii @ 2022-01-12 15:00 UTC (permalink / raw) To: Jim Porter; +Cc: 52999-done > Cc: 52999@debbugs.gnu.org > From: Jim Porter <jporterbugs@gmail.com> > Date: Wed, 5 Jan 2022 16:48:39 -0800 > > Ok, I updated this to refer to "Built-in commands in Eshell". > > Thanks for looking over the patch. Thanks, I installed this, and I'm closing the bug. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-12 15:00 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-04 1:36 bug#52999: 29.0.50; [PATCH] `eshell-eval-using-options' should follow POSIX/GNU argument conventions Jim Porter 2022-01-04 5:33 ` bug#52999: 29.0.50; [PATCH v2] " Jim Porter 2022-01-04 13:01 ` Eli Zaretskii 2022-01-04 21:09 ` bug#52999: 29.0.50; [PATCH v3] " Jim Porter 2022-01-05 14:50 ` Eli Zaretskii 2022-01-06 0:48 ` Jim Porter 2022-01-06 12:31 ` Eli Zaretskii 2022-01-08 19:58 ` Jim Porter 2022-01-08 20:10 ` Eli Zaretskii 2022-01-12 15:00 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).