unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#71438: [PATCH] Allow ping to receive optional arguments
@ 2024-06-08 15:28 TOMAS FABRIZIO ORSI
  2024-06-08 18:48 ` Peter Breton via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-09 12:02 ` Stefan Kangas
  0 siblings, 2 replies; 15+ messages in thread
From: TOMAS FABRIZIO ORSI @ 2024-06-08 15:28 UTC (permalink / raw)
  To: pbreton; +Cc: 71438


[-- Attachment #1.1: Type: text/plain, Size: 705 bytes --]

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.

[-- Attachment #1.2: Type: text/html, Size: 1026 bytes --]

[-- Attachment #2: 0001-ping-Added-optional-arguments.patch --]
[-- Type: text/x-patch, Size: 2141 bytes --]

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>
---
 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.
@@ -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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-08 15:28 bug#71438: [PATCH] Allow ping to receive optional arguments TOMAS FABRIZIO ORSI
@ 2024-06-08 18:48 ` Peter Breton via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-06-08 18:57   ` TOMAS FABRIZIO ORSI
  2024-06-09 12:02 ` Stefan Kangas
  1 sibling, 1 reply; 15+ messages in thread
From: Peter Breton via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-06-08 18:48 UTC (permalink / raw)
  To: torsi, pbreton; +Cc: 71438

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

Looks good to me 😀 

Peter 
 
  On Sat, Jun 8, 2024 at 11:28 AM, TOMAS FABRIZIO ORSI<torsi@fi.uba.ar> wrote:   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 emacsmailing 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 madea mistake.

  

[-- Attachment #2: Type: text/html, Size: 1555 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-08 18:48 ` Peter Breton via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-08 18:57   ` TOMAS FABRIZIO ORSI
  0 siblings, 0 replies; 15+ messages in thread
From: TOMAS FABRIZIO ORSI @ 2024-06-08 18:57 UTC (permalink / raw)
  To: Peter Breton; +Cc: 71438

[-- Attachment #1: Type: text/plain, Size: 196 bytes --]

Hi Peter,

Looks good to me 😀
>

I am very glad to hear this ^_^. Thank you for the quick and kind response.
What comes next? What's the standard procedure?

Best regards,
- Fabrizio

[-- Attachment #2: Type: text/html, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-08 15:28 bug#71438: [PATCH] Allow ping to receive optional arguments TOMAS FABRIZIO ORSI
  2024-06-08 18:48 ` Peter Breton via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-06-09 12:02 ` Stefan Kangas
  2024-06-09 13:47   ` Eli Zaretskii
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Kangas @ 2024-06-09 12:02 UTC (permalink / raw)
  To: TOMAS FABRIZIO ORSI, pbreton; +Cc: 71438

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.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-09 12:02 ` Stefan Kangas
@ 2024-06-09 13:47   ` Eli Zaretskii
  2024-06-09 15:03     ` TOMAS FABRIZIO ORSI
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2024-06-09 13:47 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: pbreton, torsi, 71438

> Cc: 71438@debbugs.gnu.org
> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 9 Jun 2024 05:02:51 -0700
> 
> Does this deserve to be announced in NEWS?

Yes, IMO.

> > -(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'.

I actually wonder why change the name of the option at all?

> 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?

Yes, I think we should do that, in which case the change is no longer
backward-incompatible.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-09 13:47   ` Eli Zaretskii
@ 2024-06-09 15:03     ` TOMAS FABRIZIO ORSI
  2024-06-09 15:21       ` Eli Zaretskii
  2024-06-09 15:38       ` Stefan Kangas
  0 siblings, 2 replies; 15+ messages in thread
From: TOMAS FABRIZIO ORSI @ 2024-06-09 15:03 UTC (permalink / raw)
  To: Eli Zaretskii, Stefan Kangas; +Cc: pbreton, 71438

Hello Eli and Stefan,

Thanks for the quick response.
I'll reply to both of your emails here; I hope I am not breaking Emacs
email etiquette.

Stefan Kangas said:
> Consider adding a ChangeLog entry to the commit message, see CONTRIBUTE.

Noted. I thought my patch was too small to be mentioned in ChangeLog.
I'll keep this in mind for future patches :D.
As a side note, I found the CONTRIBUTE file very helpful.
I personally found
https://www.gnu.org/software/emacs/manual/html_node/emacs/Contributing.html
to be a bit unclear.

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

Noted. I'll remove that as well. Do patches need to be PGP signed?


Eli Zaretskii said:
> I actually wonder why change the name of the option at all?

I thought that changing the name of the variable would better
indicate its intended purpose. Since, with the patch, those
variables would only be as a fallback/default (hence the name
change). In retrospect, maybe it's a bit unnecessary and the
name should be kept the same.

Eli Zaretskii said:
> Yes, I think we should do that, in which case the change is no longer
backward-incompatible.

Great! What's the procedure now? Should I upload a new patch to this
same thread with the changes you both mentioned?
As a side note, any additional notes?
Any style comments? Elisp advice? Any other function I should look into
to "imitate" the style of a good use of prefix argument?

Thank you for all the feedback,

Best regards,

- Fabrizio





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-09 15:03     ` TOMAS FABRIZIO ORSI
@ 2024-06-09 15:21       ` Eli Zaretskii
  2024-06-09 15:38       ` Stefan Kangas
  1 sibling, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2024-06-09 15:21 UTC (permalink / raw)
  To: TOMAS FABRIZIO ORSI; +Cc: pbreton, stefankangas, 71438

> From: TOMAS FABRIZIO ORSI <torsi@fi.uba.ar>
> Date: Sun, 9 Jun 2024 12:03:37 -0300
> Cc: pbreton@cs.umb.edu, 71438@debbugs.gnu.org
> 
> Stefan Kangas said:
> > Consider adding a ChangeLog entry to the commit message, see CONTRIBUTE.
> 
> Noted. I thought my patch was too small to be mentioned in ChangeLog.
> I'll keep this in mind for future patches :D.
> As a side note, I found the CONTRIBUTE file very helpful.
> I personally found
> https://www.gnu.org/software/emacs/manual/html_node/emacs/Contributing.html
> to be a bit unclear.

I suggest instead to read the file CONTRIBUTE in the top-level
directory of the Emacs source tree.

> Eli Zaretskii said:
> > Yes, I think we should do that, in which case the change is no longer
> backward-incompatible.
> 
> Great! What's the procedure now? Should I upload a new patch to this
> same thread with the changes you both mentioned?

Yes, please .

> As a side note, any additional notes?
> Any style comments?

Just one: try to keep lines shorter than 80 columns, and break longer
lines into several ones.

> Elisp advice? Any other function I should look into
> to "imitate" the style of a good use of prefix argument?

Any command that uses the fact of being invoked with a prefix argument
to modify its behavior will do.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-09 15:03     ` TOMAS FABRIZIO ORSI
  2024-06-09 15:21       ` Eli Zaretskii
@ 2024-06-09 15:38       ` Stefan Kangas
  2024-06-09 15:48         ` TOMAS FABRIZIO ORSI
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Kangas @ 2024-06-09 15:38 UTC (permalink / raw)
  To: TOMAS FABRIZIO ORSI, Eli Zaretskii; +Cc: pbreton, 71438

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

> Do patches need to be PGP signed?

We don't mind if people want to do that, but we don't ask them to.

In my experience, very few patches end up being PGP signed.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-09 15:38       ` Stefan Kangas
@ 2024-06-09 15:48         ` TOMAS FABRIZIO ORSI
  2024-06-16 21:46           ` TOMAS FABRIZIO ORSI
  0 siblings, 1 reply; 15+ messages in thread
From: TOMAS FABRIZIO ORSI @ 2024-06-09 15:48 UTC (permalink / raw)
  To: Stefan Kangas, Eli Zaretskii; +Cc: pbreton, 71438

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

Hello Stefan and Eli,

Here I added the use of a prefix argument. I tested and seems to be
working fine ^_^. As a matter of fact, I think it's even better now!
Pretty handy thing.

I tried my best to break at the 80 character column, whilst
maintaining readability.
I also went back on the variable ping-program-options'
name change. Eli raised a good point.

Would appreciate some feedback.

Best regards,

- Fabrizio

[-- Attachment #2: 0001-ping-Added-optional-arguments.patch --]
[-- Type: text/x-patch, Size: 1881 bytes --]

From 40030845d3691480f4b75f8a65ab84f345508465 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

---
 lisp/net/net-utils.el | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/lisp/net/net-utils.el b/lisp/net/net-utils.el
index 83842cd..c37d997 100644
--- a/lisp/net/net-utils.el
+++ b/lisp/net/net-utils.el
@@ -425,22 +425,29 @@ 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'
+With a \\[universal-argument] prefix arg, prompt the user for OPTIONS.
+If called interactively with no prefix arg, then `ping-program-options'
+will be used.
+If OPTIONS is not set, then `ping-program-options' will be used.
 If your system's ping continues until interrupted, you can try setting
 `ping-program-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
+           (read-string (format-prompt "Ping host" default) nil nil default))
+         (if current-prefix-arg (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-options (list host))
-	   (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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-09 15:48         ` TOMAS FABRIZIO ORSI
@ 2024-06-16 21:46           ` TOMAS FABRIZIO ORSI
  2024-06-17  1:14             ` Stefan Kangas
  0 siblings, 1 reply; 15+ messages in thread
From: TOMAS FABRIZIO ORSI @ 2024-06-16 21:46 UTC (permalink / raw)
  To: Stefan Kangas, Eli Zaretskii; +Cc: pbreton, 71438

Hello Stefan and Eli,

I am sorry to bother you again. I know you two must be very busy.

Did you have the chance to look at the patch?
The patch did not have the changes to changelog because I was
unsure if there were going to be any additional comments regarding
the e-lisp code.
Should I send a patch with the changelog modified?

Best regards,

Thank you for all of you work

- Fabrizio





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-16 21:46           ` TOMAS FABRIZIO ORSI
@ 2024-06-17  1:14             ` Stefan Kangas
  2024-06-17  2:03               ` TOMAS FABRIZIO ORSI
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Kangas @ 2024-06-17  1:14 UTC (permalink / raw)
  To: TOMAS FABRIZIO ORSI, Eli Zaretskii; +Cc: pbreton, 71438

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

> Did you have the chance to look at the patch?
> The patch did not have the changes to changelog because I was
> unsure if there were going to be any additional comments regarding
> the e-lisp code.
> Should I send a patch with the changelog modified?

Sorry for the late reply.

I think this patch (to be applied on top of yours) might provide a
slighly better experience.  I also made an attempt at improving the
docstring.  What do you think?

diff --git a/lisp/net/net-utils.el b/lisp/net/net-utils.el
index c37d9976cf2..8fb417da241 100644
--- a/lisp/net/net-utils.el
+++ b/lisp/net/net-utils.el
@@ -425,24 +425,30 @@ traceroute
      options)))

 ;;;###autoload
-(defun ping (host &optional options)
-  "Ping HOST.
-Optional argument OPTIONS sets which options will be passed to `ping-program'
-With a \\[universal-argument] prefix arg, prompt the user for OPTIONS.
-If called interactively with no prefix arg, then `ping-program-options'
-will be used.
-If OPTIONS is not set, then `ping-program-options' will be used.
-If your system's ping continues until interrupted, you can try setting
-`ping-program-options'."
+(defun ping (host &optional flags)
+  "Ping HOST using `ping-program'.
+
+The user option `ping-program-options' is passed as flags to
+`ping-program'.  With a \\[universal-argument] prefix arg, prompt the
+user for the flags to pass.
+
+When called from Lisp, the optional argument FLAGS, if non-nil, is a
+list of strings that will be passed as flags for the `ping-program'.  If
+FLAGS is nil, `ping-program-options' will be used.
+
+If your system's ping continues until interrupted, you can try using a
+prefix argument or setting `ping-program-options'."
   (interactive
    (list (let ((default (ffap-machine-at-point)))
            (read-string (format-prompt "Ping host" default) nil nil default))
-         (if current-prefix-arg (split-string (read-string
-            (format-prompt "Ping options (RET for defaults)" nil) nil
nil nil) " "))))
+         (when current-prefix-arg
+           (split-string
+            (read-string (format-prompt "Ping options" ping-program-options)
+                         nil nil ping-program-options)))))
   (let ((full-command
-	 (if (or (equal options (list "")) (not options))
+	 (if (or (equal flags (list "")) (not flags))
 	     (append ping-program-options (list host))
-	     (append options (list host)))))
+	     (append flags (list host)))))
     (net-utils-run-program
      (concat "Ping" " " host)
      (concat "** Ping ** " ping-program " ** " host)





^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-17  1:14             ` Stefan Kangas
@ 2024-06-17  2:03               ` TOMAS FABRIZIO ORSI
  2024-06-17  6:19                 ` Stefan Kangas
  0 siblings, 1 reply; 15+ messages in thread
From: TOMAS FABRIZIO ORSI @ 2024-06-17  2:03 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: pbreton, Eli Zaretskii, 71438

Hello Stefan,

> Sorry for the late reply.
No worries Stefan! I'm glad you are taking the time to help me with the patch.
Thank you for all of the work you do!

> I think this patch (to be applied on top of yours) might provide a
> slighly better experience.  I also made an attempt at improving the
> docstring.  What do you think?
The docstring is greatly improved; without a doubt.

However, I've been thinking. Both my original patch and yours
ask the user for additional flags if *any* prefix argument is
passed.
Isn't that closing the door for further additions to this
function?

Maybe, it would be better if (for instance) "0" is passed
as a prefix argument.
In that way, if any other additions were to be made,
those could use a different prefix argument (1,2,3, and so on);
thus avoiding backwards incompatibility.

So, instead of using when/if; cond could be used.

What do you think?

Best regards,

- Fabrizio





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-17  2:03               ` TOMAS FABRIZIO ORSI
@ 2024-06-17  6:19                 ` Stefan Kangas
  2024-06-19  2:17                   ` TOMAS FABRIZIO ORSI
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Kangas @ 2024-06-17  6:19 UTC (permalink / raw)
  To: TOMAS FABRIZIO ORSI; +Cc: pbreton, Eli Zaretskii, 71438

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

> However, I've been thinking. Both my original patch and yours
> ask the user for additional flags if *any* prefix argument is
> passed.
> Isn't that closing the door for further additions to this
> function?
>
> Maybe, it would be better if (for instance) "0" is passed
> as a prefix argument.
> In that way, if any other additions were to be made,
> those could use a different prefix argument (1,2,3, and so on);
> thus avoiding backwards incompatibility.

This is not a problem, and we can easily make such changes later.

One prefix argument is equivalent to 4 (C-u C-u is 16, and so on).
See (info "(elisp) Prefix Command Arguments").





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-17  6:19                 ` Stefan Kangas
@ 2024-06-19  2:17                   ` TOMAS FABRIZIO ORSI
  2024-06-20 18:45                     ` Stefan Kangas
  0 siblings, 1 reply; 15+ messages in thread
From: TOMAS FABRIZIO ORSI @ 2024-06-19  2:17 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: pbreton, Eli Zaretskii, 71438

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]

Hello Stefan and Eli,

I apologize for my late reply. I wanted to thank you both for
your work and patience.

And Stefan for teaching me the committing etiquette.

I added a "NetUtils" heading in etc/NEWS, which
I am a little hesitant about. I was torn between
creating a new Heading and using "Miscellaneous".

I am looking forward to your feedback.

Have a good week,

- Fabrizio

[-- Attachment #2: 0003-ping-can-now-receive-a-prefix-argument.patch --]
[-- Type: text/x-patch, Size: 2827 bytes --]

From 3da7ba70ea2c6f2d9c25acd4a770a3e31b9dea3a 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 can now receive a prefix argument

ping will check for a prefix argument. If given,
the user will be prompted for flags to pass to ping.
* etc/NEWS: Document the change
* lisp/net/net-utils.el (ping):

Copyright-paperwork-exempt: yes
Signed-off-by: Tomas Fabrizio Orsi <torsi@fi.uba.ar>
---
 etc/NEWS              |  2 ++
 lisp/net/net-utils.el | 31 ++++++++++++++++++++++---------
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d6a8fa7..74f7ed4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1907,6 +1907,8 @@ The following new XML schemas are now supported:
 - Nuget packages config file
 
 ** color.el now supports the Oklab color representation.
+** Netutils
+*** 'ping' can now receive a prefix argument for additional options
 
 \f
 * New Modes and Packages in Emacs 30.1
diff --git a/lisp/net/net-utils.el b/lisp/net/net-utils.el
index 83842cd..fe68054 100644
--- a/lisp/net/net-utils.el
+++ b/lisp/net/net-utils.el
@@ -425,22 +425,35 @@ This variable is only used if the variable
      options)))
 
 ;;;###autoload
-(defun ping (host)
-  "Ping HOST.
-If your system's ping continues until interrupted, you can try setting
-`ping-program-options'."
+(defun ping (host &optional flags)
+  "Ping HOST using `ping-program'.
+
+The user option `ping-program-options' is passed as flags to
+`ping-program'.  With a \\[universal-argument] prefix arg, prompt the
+user for the flags to pass.
+
+When called from Lisp, the optional argument FLAGS, if non-nil, is a
+list of strings that will be passed as flags for the `ping-program'.  If
+FLAGS is nil, `ping-program-options' will be used.
+
+If your system's ping continues until interrupted, you can try using a
+prefix argument or setting `ping-program-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
+           (read-string (format-prompt "Ping host" default) nil nil default))
+         (when current-prefix-arg
+           (split-string
+            (read-string (format-prompt "Ping options" ping-program-options)
+                         nil nil ping-program-options)))))
+  (let ((full-command
+	 (if (or (equal flags (list "")) (not flags))
 	     (append ping-program-options (list host))
-	   (list host))))
+	   (append flags (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


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* bug#71438: [PATCH] Allow ping to receive optional arguments
  2024-06-19  2:17                   ` TOMAS FABRIZIO ORSI
@ 2024-06-20 18:45                     ` Stefan Kangas
  0 siblings, 0 replies; 15+ messages in thread
From: Stefan Kangas @ 2024-06-20 18:45 UTC (permalink / raw)
  To: TOMAS FABRIZIO ORSI; +Cc: pbreton, Eli Zaretskii, 71438

close 71438 30.1
thanks

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

> Hello Stefan and Eli,
>
> I apologize for my late reply. I wanted to thank you both for
> your work and patience.
>
> And Stefan for teaching me the committing etiquette.
>
> I added a "NetUtils" heading in etc/NEWS, which
> I am a little hesitant about. I was torn between
> creating a new Heading and using "Miscellaneous".
>
> I am looking forward to your feedback.

I've installed it on master as commit 72f2b01e318 with some minor
touch-ups.  I'm consequently closing this bug.

This brings us to the limit of what we can accept from you without a
copyright assignment to the Free Software Foundation.  Please let us
know if you plan any further contributions, and we can send you the form
to get you started with the copyright assignment process.

Thanks for contributing to Emacs!





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-06-20 18:45 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-08 15:28 bug#71438: [PATCH] Allow ping to receive optional arguments TOMAS FABRIZIO ORSI
2024-06-08 18:48 ` Peter Breton via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-08 18:57   ` TOMAS FABRIZIO ORSI
2024-06-09 12:02 ` Stefan Kangas
2024-06-09 13:47   ` Eli Zaretskii
2024-06-09 15:03     ` TOMAS FABRIZIO ORSI
2024-06-09 15:21       ` Eli Zaretskii
2024-06-09 15:38       ` Stefan Kangas
2024-06-09 15:48         ` TOMAS FABRIZIO ORSI
2024-06-16 21:46           ` TOMAS FABRIZIO ORSI
2024-06-17  1:14             ` Stefan Kangas
2024-06-17  2:03               ` TOMAS FABRIZIO ORSI
2024-06-17  6:19                 ` Stefan Kangas
2024-06-19  2:17                   ` TOMAS FABRIZIO ORSI
2024-06-20 18:45                     ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).