GNU bug report logs - #52999
29.0.50; [PATCH] `eshell-eval-using-options' should follow POSIX/GNU argument conventions

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 52999 <at> debbugs.gnu.org
Subject: bug#52999: 29.0.50; [PATCH v2] `eshell-eval-using-options' should follow POSIX/GNU argument conventions
Date: Tue, 04 Jan 2022 15:01:50 +0200

> From: Jim Porter <jporterbugs <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.