[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow
From: |
Jim Porter |
Subject: |
bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions |
Date: |
Tue, 4 Jan 2022 13:09:29 -0800 |
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.)
0001-Follow-POSIX-GNU-argument-conventions-for-eshell-eva.patch
Description: Text document
- bug#52999: 29.0.50; [PATCH] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Jim Porter, 2022/01/03
- bug#52999: 29.0.50; [PATCH v2] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Jim Porter, 2022/01/04
- bug#52999: 29.0.50; [PATCH v2] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Eli Zaretskii, 2022/01/04
- bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions,
Jim Porter <=
- bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Eli Zaretskii, 2022/01/05
- bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Jim Porter, 2022/01/05
- bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Eli Zaretskii, 2022/01/06
- bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Jim Porter, 2022/01/08
- bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Eli Zaretskii, 2022/01/08
- bug#52999: 29.0.50; [PATCH v3] `eshell-eval-using-options' should follow POSIX/GNU argument conventions, Eli Zaretskii, 2022/01/12