Thanks, I've attached an updated patch. On 1/4/2022 5:01 AM, Eli Zaretskii wrote: >> From: Jim Porter >> 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.)