bug#71438: [PATCH] Allow ping to receive optional arguments

[Stefan Kangas <stefankangas@gmail.com>]

TOMAS FABRIZIO ORSI <torsi@fi.uba.ar> writes:

> Hi! This small patch makes it so that the "ping" interactive
> function can receive optional arguments.
> This allows the caller to change its argument every time. If no
> arguments are passed, the default variable will be used.
>
> Prior to this (if I am not mistaken), one had to change the
> variable each time.
>
> I would love to get some feedback! Thanks in advance.
>
> Version: GNU Emacs 29.3 (build 1, x86_64-pc-linux-gnu,
> GTK+ Version 3.24.41, cairo version 1.18.0) of 2024-05-18
>
> PS: This is my first time sending an email to the emacs
> mailing list. I CC'd  bug-gnu-emacs@gnu.org because it
> was the address that the CONTRIBUTE file stated one
> should send patches to. I apologize in advance if I made
> a mistake.

Thanks for your contribution to Emacs!

This is the right list to send your patch to.

> From de527784bb1f6e60a65291e5ab798328fd18c8e0 Mon Sep 17 00:00:00 2001
> From: Tomas Fabrizio Orsi <torsi@fi.uba.ar>
> Date: Sat, 8 Jun 2024 12:11:18 -0300
> Subject: [PATCH] ping: Added optional arguments
>
> Signed-off-by: Tomas Fabrizio Orsi <torsi@fi.uba.ar>

Consider adding a ChangeLog entry to the commit message, see CONTRIBUTE.

Note that we don't usually use "Signed-off-by" trailers so that can
probably be removed as redundant.

Does this deserve to be announced in NEWS?

> ---
>  lisp/net/net-utils.el | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/lisp/net/net-utils.el b/lisp/net/net-utils.el
> index 83842cd..6f69326 100644
> --- a/lisp/net/net-utils.el
> +++ b/lisp/net/net-utils.el
> @@ -78,7 +78,7 @@
>
>  ;; On GNU/Linux and Irix, the system's ping program seems to send packets
>  ;; indefinitely unless told otherwise
> -(defcustom ping-program-options
> +(defcustom ping-program-default-options
>    (and (eq system-type 'gnu/linux)
>         (list "-c" "4"))
>    "Options for the ping program.

This needs an `define-obsolete-variable-alias'.

> @@ -425,22 +425,25 @@ This variable is only used if the variable
>       options)))
>
>  ;;;###autoload
> -(defun ping (host)
> +(defun ping (host &optional options)
>    "Ping HOST.
> +Optional argument OPTIONS sets which options will be passed to `ping-program'
> +If OPTIONS is not set, then `ping-program-default-options' will be used.
>  If your system's ping continues until interrupted, you can try setting
> -`ping-program-options'."
> +`ping-program-default-options'."
>    (interactive
>     (list (let ((default (ffap-machine-at-point)))
> -           (read-string (format-prompt "Ping host" default) nil nil default))))
> -  (let ((options
> -	 (if ping-program-options
> -	     (append ping-program-options (list host))
> -	   (list host))))
> +           (read-string (format-prompt "Ping host" default) nil nil default))
> +         (split-string (read-string (format-prompt "Ping options (RET for defaults)" nil) nil nil nil) " ")))
> +  (let ((full-command
> +	 (if (or (equal options (list "")) (not options))
> +	     (append ping-program-default-options (list host))
> +	     (append options (list host)))))
>      (net-utils-run-program
>       (concat "Ping" " " host)
>       (concat "** Ping ** " ping-program " ** " host)
>       ping-program
> -     options)))
> +     full-command)))
>
>  ;;;###autoload
>  (defun nslookup-host (host &optional name-server)
> --
> 2.44.2

There is an extra RET before you can ping, so the change is
backwards-incompatible.  I rarely if ever use `M-x ping`, so I can't
give a very informed opinion here, but:

- Perhaps we should make it prompt for options only with a prefix
  command?

- Or an option that reverts back to the old behaviour (or enables the
  new)?

- Or is the new behaviour simply more useful and should be enabled
  unconditionally?  At the very least, this needs some kind of
  rationale.

Let's see if anyone else has any thoughts here.