bug#53293: 29.0.50; [PATCH] `eshell-eval-using-options' should report error for unrecognized option, even with no :external

[Jim Porter]

Currently, `eshell-eval-using-options' reports an error if the user 
passes an unrecognized option, but only if 1) the :external keyword has 
been set and 2) the external program can't actually be found. If you'd 
like to see this in action, you can run "echo -Z hi" in Eshell. It just 
silently discards the "-Z". Editing `eshell/echo' to include `:external 
"nonexist"' in its option spec instead results in an error being 
reported to the user.

I think this might be simply an oversight in the implementation. The 
documentation of `eshell--process-option' states:

  If no matching [option] 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.

I would interpret this to mean the following:

  (if (and (not handler-found)
           external-cmd-available)
      (call-external)
    (error "unrecognized option"))

Attached is a patch that ensures unrecognized options report an error 
even when there's no :external command. This *is* a slightly 
incompatible change though. The following Eshell commands use 
`eshell-eval-using-options' with no :external command, so they'll start 
erroring out if you pass unrecognized arguments to them with this patch:

  addpath
  echo
  history
  source / .
  su
  sudo
  umask

Despite this incompatibility, I still think this is the right change to 
make for all these commands. If a user passes unrecognized options to 
any of these, they should be informed of that fact. For example, `su' is 
used in Eshell to invoke TRAMP's su method. It would likely be an 
unpleasant surprise for a user if they tried to pass flags to it that 
only work with /bin/su, only for those options to be silently ignored. 
One of the nastiest parts of the pre-patch behavior is that something 
like "su -c CMD" simply drops the "-c", which results in CMD being 
treated as the USER instead.

I'm not sure if this should be explicitly called out in the manual or 
whether it warrants a NEWS entry. To me, it just seems like a bug, and 
one that was already documented as working the way it does with this 
patch applied. That said, if others think this warrants some more 
documentation, I'm happy to add some.

0001-Raise-an-error-from-eval-eval-using-options-for-unkn.patch
Description: Text document