GNU bug report logs - #25426
[PATCH] in net-utils.el several DNS query functions now take optional name server argument; also, `dig-program-options' added

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 25426 in the body.
You can then email your comments to 25426 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] in net-utils.el several DNS query functions now take optional
 name server argument; also, `dig-program-options' added 
Date: Thu, 12 Jan 2017 09:03:02 -0500

[Message part 1 (text/plain, inline)]

- In a git-clone'd "lisp/net/net-utils.el" I modified several
interactive DNS-querying functions (nslookup-host,
dns-lookup-host, run-dig) to allow specifying a particular name
server to use where before you couldn't. This is possible
non-interactively as well as interactively with a prefix argument.

- Also, a new variable `dig-program-options' is defined to allow
users to set options for use with `run-dig'. This is in line with
the current `nslookup-program-options' and
`dns-lookup-program-options' and is necessary for the new
definition of `run-dig' to function.

- The `run-dig' function no longer needs the auto-loaded
`ffap-string-at-point' function, removing one dependency on
ffap.el 

For all users, and especially those unaware of the changes, there
should be no discernible difference in functionality when used
exactly as before.

My changelog and diff are attached.

[changelog_net-utils (application/octet-stream, attachment)]

[diff_net-utils (application/octet-stream, attachment)]

Message #8 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: 25426 <at> debbugs.gnu.org
Subject: Re: Copyright Assignment
Date: Mon, 13 Feb 2017 22:15:08 -0500

I recently realized that I neglected to include in my patch
submission information regarding copyright assignment. I haven't
filled out the required paperwork to assign copyright to the FSF
so if that is the only thing standing in the way of accepting the
patch then I will gladly fill it out should a dev send it my
way.

Should the patch not be merged, I'll be sure to include the above
information in any future patch submissions (and make them better
as well!).

Regards,
Andrew Robbins

Message #11 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrew Robbins <contact <at> andrewrobbins.info>
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: Copyright Assignment
Date: Tue, 14 Feb 2017 18:23:12 +0200

> From: Andrew Robbins <contact <at> andrewrobbins.info>
> Date: Mon, 13 Feb 2017 22:15:08 -0500
> 
> I recently realized that I neglected to include in my patch
> submission information regarding copyright assignment. I haven't
> filled out the required paperwork to assign copyright to the FSF
> so if that is the only thing standing in the way of accepting the
> patch then I will gladly fill it out should a dev send it my
> way.

Yes, your contribution is large enough to require legal paperwork.

I will send you the copyright assignment form off-list.

Thanks.

Message #14 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: 25426 <at> debbugs.gnu.org
Subject: Copyright Assignment Completed
Date: Fri, 05 May 2017 22:40:18 -0400

I completed the copyright assignment paperwork a couple of months
ago but forgot to follow-up on here afterward.

So, uh, here's that (late) follow-up.

Message #17 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrew Robbins <contact <at> andrewrobbins.info>
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: Copyright Assignment Completed
Date: Sat, 06 May 2017 10:16:58 +0300

> From: Andrew Robbins <contact <at> andrewrobbins.info>
> Date: Fri, 05 May 2017 22:40:18 -0400
> 
> I completed the copyright assignment paperwork a couple of months
> ago but forgot to follow-up on here afterward.
> 
> So, uh, here's that (late) follow-up.

Thanks.

Would people please look at the proposed patch and comment?  I will
push to master in a few days if there are no comments.

Message #20 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: contact <at> andrewrobbins.info
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: Copyright Assignment Completed
Date: Sat, 06 May 2017 10:50:31 +0300

> Date: Sat, 06 May 2017 10:16:58 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 25426 <at> debbugs.gnu.org
> 
> Would people please look at the proposed patch and comment?  I will
> push to master in a few days if there are no comments.

Some minor comments of my own:

> 2017-01-12  Andrew Robbins  <contact <at> andrewrobbins.info>
> 
> 	* lisp/net/net-utils.el (ffap-string-at-point): Removed due to
> 	'net-utils-machine-at-point' obviating this autoloaded function.
> 	(dig-program-options): New customization variable.
> 	(nslookup-host, dns-lookup-host, run-dig): Can now specify optional
> 	name server argument interactively (by prefix arg) & noninteractively.

Please mention the bug number in the log message, for reference.

> +(defcustom dig-program-options nil
> +  "Options for the dig program."
> +  :group 'net-utils
> +  :type '(repeat string))

When you introduce new defcustoms, please provide a :version tag whose
value is the first Emacs version where this defcustom will be
available (26.1 in this case).

> +(defun nslookup-host (host &optional name-server)
> +  "Look up the DNS information for HOST (name or IP address).
> +When called interactively, DNS resolver NAME-SERVER may be specified
> +by passing the function a prefix argument."

Our standard wording for such optional behavior is like this:

    Look up the DNS information for HOST (name or IP address).
  Optional argument NAME-SERVER says which server to use for
  DNS resolution.
  Interactively, prompt for NAME-SERVER if  invoked with prefix argument.

> +(defun dns-lookup-host (host &optional name-server)
> +  "Look up the DNS information for HOST (name or IP address).
> +When called interactively, DNS resolver NAME-SERVER may be specified
> +by passing the function a prefix argument."

Same here.

> +(defun run-dig (host &optional name-server)
> +  "Look up the DNS information for HOST (name or IP address).
> +When called interactively, DNS resolver NAME-SERVER may be specified
> +by passing the function a prefix argument."

And here.

Finally, please add a NEWS entry to describe these new features.

Thanks for working on this.

Message #23 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: Copyright Assignment Completed
Date: Sat, 06 May 2017 07:05:03 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sat, 06 May 2017 10:16:58 +0300
>> From: Eli Zaretskii <eliz <at> gnu.org>
>> Cc: 25426 <at> debbugs.gnu.org
>> 
>> Would people please look at the proposed patch and comment?  I will
>> push to master in a few days if there are no comments.
>
> Some minor comments of my own:

> Please mention the bug number in the log message, for reference.
>
>> +(defcustom dig-program-options nil
>> +  "Options for the dig program."
>> +  :group 'net-utils
>> +  :type '(repeat string))
>
> When you introduce new defcustoms, please provide a :version tag whose
> value is the first Emacs version where this defcustom will be
> available (26.1 in this case).

>> +(defun nslookup-host (host &optional name-server)
>> +  "Look up the DNS information for HOST (name or IP address).
>> +When called interactively, DNS resolver NAME-SERVER may be specified
>> +by passing the function a prefix argument."
>
> Our standard wording for such optional behavior is like this:
>
>     Look up the DNS information for HOST (name or IP address).
>   Optional argument NAME-SERVER says which server to use for
>   DNS resolution.
>   Interactively, prompt for NAME-SERVER if  invoked with prefix argument.
>
>> +(defun dns-lookup-host (host &optional name-server)
>
> Same here.
>
>> +(defun run-dig (host &optional name-server)
>
> And here.

> Finally, please add a NEWS entry to describe these new features.
>
> Thanks for working on this.

No problem--I'll have those changes made and posted back here
within the next 24 hours.  If anyone else gives additional
feedback, I'll include those suggestions in the revised patch as
well.

'Til then.

Message #26 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Andrew Robbins <contact <at> andrewrobbins.info>
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: [PATCH] in net-utils.el several DNS query functions
 now take optional name server argument; also, `dig-program-options' added
Date: Sat, 06 May 2017 08:04:01 -0400

Andrew Robbins <contact <at> andrewrobbins.info> writes:

> -   (list (read-from-minibuffer "Lookup host: " (net-utils-machine-at-point))))
> +   (if current-prefix-arg
> +       (list
> +        (read-from-minibuffer "Look up host: " (net-utils-machine-at-point))
> +        (read-from-minibuffer "Name server: "))
> +     (list
> +      (read-from-minibuffer "Look up host: " (net-utils-machine-at-point)))))

You could avoid repeating the "Look up host" code like this:

    (list (read-from-minibuffer "Look up host: " (net-utils-machine-at-point))
          (if current-prefix-arg (read-from-minibuffer "Name server: ")))

> -	 (if nslookup-program-options
> -	     (append nslookup-program-options (list host))
> -	   (list host))))
> +	 (if name-server
> +	     (append nslookup-program-options (list host name-server))
> +	   (append nslookup-program-options (list host)))))

Similarly

    (append nslookup-program-options (list host)
            (if name-sever (list name-server)))

> -      (mapconcat 'identity
> -		(list "Nslookup" host nslookup-program)
> -		" ** "))
> +	     (mapconcat 'identity
> +		        (list "Nslookup" host nslookup-program)
> +		        " ** "))

This is just a whitespace change, right?


Same comments again for dns-lookup-host and run-dig changes.

Message #29 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: npostavs <at> users.sourceforge.net
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: [PATCH] in net-utils.el several DNS query functions
 now take optional name server argument; also, `dig-program-options' added
Date: Sun, 07 May 2017 04:02:47 -0400

npostavs <at> users.sourceforge.net writes:

> You could avoid repeating the "Look up host" code like this:
>
>     (list (read-from-minibuffer "Look up host: " (net-utils-machine-at-point))
>           (if current-prefix-arg (read-from-minibuffer "Name server: ")))

Nice.  I thought it looked a bit weird when I wrote that bit, but
your simplification is better.

> Similarly
>
>     (append nslookup-program-options (list host)
>             (if name-sever (list name-server)))

Same as above.

>> -      (mapconcat 'identity
>> -		(list "Nslookup" host nslookup-program)
>> -		" ** "))
>> +	     (mapconcat 'identity
>> +		        (list "Nslookup" host nslookup-program)
>> +		        " ** "))
>
> This is just a whitespace change, right?

Yes.  I dropped the whitespace changes in this newest patch.

Message #32 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: 25426 <at> debbugs.gnu.org
Subject: [PATCH] 26.1; Revised patch for bug#25426
Date: Sun, 07 May 2017 04:36:21 -0400

[Message part 1 (text/plain, inline)]

Attached are the diffs for 'NEWS', 'ChangeLog', and
'net-utils.el'.  I tested these again after revision, so the new
functions and user-option should work being merged in as-is.

If there's anything else that needs changing, let me know.
The NEWS entry in particular may or may not be complete since I'm
not really sure if it needs the '---' or '+++' (and manual entry).

Thanks

[ChangeLog (text/x-diff, attachment)]

[net-utils.el.NEW (text/x-diff, attachment)]

[NEWS (text/x-diff, attachment)]

Message #35 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: Andrew Robbins <contact <at> andrewrobbins.info>
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: [PATCH] 26.1; Revised patch for bug#25426
Date: Sun, 07 May 2017 04:40:42 -0400

[Message part 1 (text/plain, inline)]

Andrew Robbins <contact <at> andrewrobbins.info> writes:

> Attached are the diffs for 'NEWS', 'ChangeLog', and
> 'net-utils.el'.

Oops, attached the wrong files.  THESE are the diffs.

[ChangeLog.diff (text/x-diff, attachment)]

[net-utils.diff (text/x-diff, attachment)]

[NEWS.diff (text/x-diff, attachment)]

Message #38 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Andrew Robbins <contact <at> andrewrobbins.info>
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: [PATCH] 26.1; Revised patch for bug#25426
Date: Sun, 07 May 2017 10:10:08 -0400

Andrew Robbins <contact <at> andrewrobbins.info> writes:

>    (let ((options
> -	 (if nslookup-program-options
> -	     (append nslookup-program-options (list host))
> -	   (list host))))
> -    (net-utils-run-program
> +         (append nslookup-program-options (list host)
> +                 (if name-server (list name-server)))))
> +  (net-utils-run-program
>       "Nslookup"
>       (concat "** "
>        (mapconcat 'identity

The indentation on (net-utils-run-program...) seems to have got messed
up.  Otherwise looks good to me.

Message #41 received at 25426 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Robbins <contact <at> andrewrobbins.info>
To: npostavs <at> users.sourceforge.net
Cc: 25426 <at> debbugs.gnu.org
Subject: Re: bug#25426: [PATCH] 26.1; Revised patch for bug#25426
Date: Sun, 07 May 2017 16:27:44 -0400

[Message part 1 (text/plain, inline)]

npostavs <at> users.sourceforge.net writes:

> Andrew Robbins <contact <at> andrewrobbins.info> writes:
>
>>    (let ((options
>> -	 (if nslookup-program-options
>> -	     (append nslookup-program-options (list host))
>> -	   (list host))))
>> -    (net-utils-run-program
>> +         (append nslookup-program-options (list host)
>> +                 (if name-server (list name-server)))))
>> +  (net-utils-run-program
>>       "Nslookup"
>>       (concat "** "
>>        (mapconcat 'identity
>
> The indentation on (net-utils-run-program...) seems to have got messed
> up.  Otherwise looks good to me.

Ah, good catch.  Here's the final diff if there aren't any other
modifications that need to be made:

[net-utils.final.diff (text/x-diff, attachment)]

Message #46 received at 25426-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrew Robbins <contact <at> andrewrobbins.info>
Cc: 25426-done <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net
Subject: Re: bug#25426: [PATCH] 26.1; Revised patch for bug#25426
Date: Fri, 12 May 2017 11:37:18 +0300

> From: Andrew Robbins <contact <at> andrewrobbins.info>
> Date: Sun, 07 May 2017 16:27:44 -0400
> Cc: 25426 <at> debbugs.gnu.org
> 
> Ah, good catch.  Here's the final diff if there aren't any other
> modifications that need to be made:

Thanks, installed.

Previous Next

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