All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
To: Kevin Locke <kevin@kevinlocke.name>
Cc: "John W. Linville" <linville@tuxdriver.com>,
	<netdev@vger.kernel.org>,
	jesse.brandeburg@intel.com
Subject: Re: [PATCH] ethtool: Add bash-completion script
Date: Thu, 11 Apr 2019 09:14:38 -0700	[thread overview]
Message-ID: <20190411091438.00007a81@intel.com> (raw)
In-Reply-To: <deff591b261b01f1be90dec14eaffac468fed638.1554689977.git.kevin@kevinlocke.name>

On Sun, 7 Apr 2019 20:19:37 -0600
Kevin Locke <kevin@kevinlocke.name> wrote:

> To aid users constructing a valid ethtool invocation, create a
> [bash-completion] script to provide [programmable completion] of ethtool
> arguments.  It supports all current command options.
> 
> The script is placed in shell-completion/bash and installed to
> completionsdir from pkg-config for bash-completion, similar to [kmod].
> It requires pkg-config 0.18 or later to be installed on the build
> system which runs aclocal (for the PKG_CHECK_MODULES m4 macro).
> 
> Note: In [scop/bash-completion#289] the bash-completion maintainer
> suggested shipping this completion with ethtool rather than
> bash-completion, due to assumptions about the ethtool command-line
> format made by the script.  That pull request also contains an extensive
> test suite in Python which is not included in this commit, but may be
> ported to a format suitable for inclusion if there is sufficient
> interest and agreement about how to achieve that.
> 
> [bash-completion]: https://github.com/scop/bash-completion
> [programmable completion]: https://www.gnu.org/software/bash/manual/html_node/Programmable-Completion.html
> [kmod]: https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/tree/
> [scop/bash-completion#289]: https://github.com/scop/bash-completion/pull/289
> 
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>

Thank you! I think this is super useful, and I agree FWIW that it
should be part of ethtool.  I suspect that it needs to be installed by
any package manager as part of ethtool install, into the right
directory.  Could be a followup patch?

And the only (minor) complaint I have about your patch is that the
commit message doesn't show how to install it (basically just copy
ethtool file from this patch to
f.e. /usr/share/bash-completion/completions/)

I did a quick touch test of it and it seemed to be completing ethtool
commands which made me really happy, especially the network interface
names which are so long.

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

And now I'm going to go and tell everyone I work with about this
patch. :-)


  reply	other threads:[~2019-04-11 16:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-08  2:19 [PATCH] ethtool: Add bash-completion script Kevin Locke
2019-04-11 16:14 ` Jesse Brandeburg [this message]
2019-04-11 16:47   ` Kevin Locke
2019-04-11 17:39 ` [PATCH v2] " Kevin Locke
2019-04-16 18:37   ` John W. Linville
2019-04-17  2:53     ` Kevin Locke
2019-04-18 16:05       ` John W. Linville
2019-04-20  0:16   ` [PATCH v3] " Kevin Locke
2019-04-24 19:54     ` John W. Linville

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190411091438.00007a81@intel.com \
    --to=jesse.brandeburg@intel.com \
    --cc=kevin@kevinlocke.name \
    --cc=linville@tuxdriver.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.