diff mbox

[IPROUTE] : Fix up "ip link" help text

Message ID 48EBCE59.3080401@trash.net
State Not Applicable, archived
Headers show

Commit Message

Patrick McHardy Oct. 7, 2008, 9:02 p.m. UTC 
Arkadiusz Miskiewicz wrote:
> Runtime dependand help doesn't look sane. Help should always show all possible 
> options/command while trying to use unsupported option should display error 
> message.
>   

That makes sense. But don't complain to me about the quality
of the error messages afterwards please :)
commit 8c6321466df9a452ac58e09d32edd6c5b4ea0d17
Author: Patrick McHardy <kaber@trash.net>
Date:   Tue Oct 7 23:00:59 2008 +0200

    [IPROUTE]: Fix up "ip link" help text
    
    Add help for "ip link add". Also fix up the "ip link set" help text:
    
    - the flags are not mutually exclusive as suggested by the current text,
      up/down states and on/off are mutually exclusive.
    
    - txqueuelen, name etc. have nothing to do with the flags and on/off states
      at all and are certainly not mutually exclusive, fix formatting and make
      it conform the other helptexts better.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

Comments

stephen hemminger Oct. 7, 2008, 9:18 p.m. UTC | #1 
On Tue, 07 Oct 2008 23:02:17 +0200
Patrick McHardy <kaber@trash.net> wrote:

> Arkadiusz Miskiewicz wrote:
> > Runtime dependand help doesn't look sane. Help should always show all possible 
> > options/command while trying to use unsupported option should display error 
> > message.
> >   
> 
> That makes sense. But don't complain to me about the quality
> of the error messages afterwards please :)
> 
> 

I had a better message set in works. will merge mine and patrick's
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
stephen hemminger Oct. 13, 2008, 2:21 p.m. UTC | #2 
On Tue, 07 Oct 2008 23:02:17 +0200
Patrick McHardy <kaber@trash.net> wrote:

> Arkadiusz Miskiewicz wrote:
> > Runtime dependand help doesn't look sane. Help should always show all possible 
> > options/command while trying to use unsupported option should display error 
> > message.
> >   
> 
> That makes sense. But don't complain to me about the quality
> of the error messages afterwards please :)
> 
> 

I applied a variant of Patrick's original patch. I added text for delete, and
avoided some code movement.  The help text should only show what is supported
otherwise there will be user complaints.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Greear Oct. 13, 2008, 4:36 p.m. UTC | #3 
Stephen Hemminger wrote:
> On Tue, 07 Oct 2008 23:02:17 +0200
> Patrick McHardy <kaber@trash.net> wrote:
>
>   
>> Arkadiusz Miskiewicz wrote:
>>     
>>> Runtime dependand help doesn't look sane. Help should always show all possible 
>>> options/command while trying to use unsupported option should display error 
>>> message.
>>>   
>>>       
>> That makes sense. But don't complain to me about the quality
>> of the error messages afterwards please :)
>>
>>
>>     
>
> I applied a variant of Patrick's original patch. I added text for delete, and
> avoided some code movement.  The help text should only show what is supported
> otherwise there will be user complaints.
>   
I personally dislike that.  I think the tool show should everything that 
it *could* do assuming
proper kernel, etc.  Then, if it turns out the current running kernel 
can't do that, it could just
give an error:  "This feature not supported in your current kernel.  See 
[url] for info on when
features were added to the kernel."

That way, the user can see all the possible goodness out there, and 
perhaps be inclined to upgrade
to a later kernel if that gives them a feature they'd like.

Thanks,
Ben

> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
stephen hemminger Oct. 13, 2008, 6:07 p.m. UTC | #4 
On Mon, 13 Oct 2008 19:17:00 +0200
Arkadiusz Miskiewicz <arekm@maven.pl> wrote:

> On Monday 13 of October 2008, Stephen Hemminger wrote:
> > On Tue, 07 Oct 2008 23:02:17 +0200
> >
> > Patrick McHardy <kaber@trash.net> wrote:
> > > Arkadiusz Miskiewicz wrote:
> > > > Runtime dependand help doesn't look sane. Help should always show all
> > > > possible options/command while trying to use unsupported option should
> > > > display error message.
> > >
> > > That makes sense. But don't complain to me about the quality
> > > of the error messages afterwards please :)
> >
> > I applied a variant of Patrick's original patch. I added text for delete,
> > and avoided some code movement.  The help text should only show what is
> > supported otherwise there will be user complaints.
> 
> Too bad. 
> 
> Help should show all options while trying to use unsupported ones should 
> yeld "unsupported"/some error message.
> 
> Will ipv6 options etc, etc also be hidden when ipv6 not available?
> 


The options won't show on older kernels. It is not a kernel config option
difference, it is old legacy code, like back to 2.4 where the API's just
aren't there.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/ip/iplink.c b/ip/iplink.c
index 6e9ac71..781fbd2 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -41,19 +41,30 @@  static void usage(void) __attribute__((noreturn));
 
 void iplink_usage(void)
 {
-	fprintf(stderr, "Usage: ip link set DEVICE { up | down |\n");
-	fprintf(stderr, "	                     arp { on | off } |\n");
-	fprintf(stderr, "	                     dynamic { on | off } |\n");
-	fprintf(stderr, "	                     multicast { on | off } |\n");
-	fprintf(stderr, "	                     allmulticast { on | off } |\n");
-	fprintf(stderr, "	                     promisc { on | off } |\n");
-	fprintf(stderr, "	                     trailers { on | off } |\n");
-	fprintf(stderr, "	                     txqueuelen PACKETS |\n");
-	fprintf(stderr, "	                     name NEWNAME |\n");
-	fprintf(stderr, "	                     address LLADDR | broadcast LLADDR |\n");
-	fprintf(stderr, "	                     mtu MTU }\n");
-	fprintf(stderr, "	                     netns PID }\n");
+	fprintf(stderr, "Usage: ip link add [ name ] NAME\n");
+	fprintf(stderr, "                   [ link DEV ]\n");
+	fprintf(stderr, "                   [ txqueuelen PACKETS ]\n");
+	fprintf(stderr, "                   [ address LLADDR ]\n");
+	fprintf(stderr, "                   [ broadcast LLADDR ]\n");
+	fprintf(stderr, "                   [ mtu MTU ]\n");
+	fprintf(stderr, "                   type TYPE [ ARGS ]\n");
+	fprintf(stderr, "\n");
+	fprintf(stderr, "       ip link set DEVICE [ { up | down } ]\n");
+	fprintf(stderr, "	                  [ arp { on | off } ]\n");
+	fprintf(stderr, "	                  [ dynamic { on | off } ]\n");
+	fprintf(stderr, "	                  [ multicast { on | off } ]\n");
+	fprintf(stderr, "	                  [ allmulticast { on | off } ]\n");
+	fprintf(stderr, "	                  [ promisc { on | off } ]\n");
+	fprintf(stderr, "	                  [ trailers { on | off } ]\n");
+	fprintf(stderr, "	                  [ txqueuelen PACKETS ]\n");
+	fprintf(stderr, "	                  [ name NEWNAME ]\n");
+	fprintf(stderr, "	                  [ address LLADDR ]\n");
+	fprintf(stderr, "	                  [ broadcast LLADDR ]\n");
+	fprintf(stderr, "	                  [ mtu MTU ]\n");
+	fprintf(stderr, "	                  [ netns PID ]\n");
 	fprintf(stderr, "       ip link show [ DEVICE ]\n");
+	fprintf(stderr, "\n");
+	fprintf(stderr, "TYPE := { vlan | veth | dummy | ifb | macvlan }\n");
 	exit(-1);
 }