[linux-lvm] patch for separator option

[linux-lvm] patch for separator option

[Arvin Schnell]

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


Hi,

I realized that the -s/--short option for vgdisplay doesn't work
in lvm 2.2.00.31 (but also older versions).  That's because the
option --separator also gets the short option 's' in args.h.

Attached is a patch.

ciao Arvin

-- 
Dipl.-Phys. Arvin Schnell
SUSE LINUX Products GmbH
Research & Development
email: arvin@suse.de

[-- Attachment #2: lvm2-args.diff --]
[-- Type: text/plain, Size: 866 bytes --]

? lvm
Index: args.h
===================================================================
RCS file: /cvs/lvm2/LVM2/tools/args.h,v
retrieving revision 1.36
diff -u -w -r1.36 args.h
--- args.h	15 Jun 2004 17:23:49 -0000	1.36
+++ args.h	14 Dec 2004 09:28:52 -0000
@@ -43,6 +43,7 @@
 arg(minor_ARG, '\0', "minor", minor_arg)
 arg(type_ARG, '\0', "type", segtype_arg)
 arg(alloc_ARG, '\0', "alloc", alloc_arg)
+arg(separator_ARG, '\0', "separator", string_arg)
 
 /* Allow some variations */
 arg(resizable_ARG, '\0', "resizable", yes_no_arg)
@@ -99,7 +100,6 @@
 arg(resizefs_ARG, 'r', "resizefs", NULL)
 arg(reset_ARG, 'R', "reset", NULL)
 arg(physicalextentsize_ARG, 's', "physicalextentsize", size_mb_arg)
-arg(separator_ARG, 's', "separator", string_arg)
 arg(stdin_ARG, 's', "stdin", NULL)
 arg(snapshot_ARG, 's', "snapshot", NULL)
 arg(short_ARG, 's', "short", NULL)

Re: [linux-lvm] patch for separator option

[Arvin Schnell]

On Tue, Dec 14, 2004 at 10:38:50AM +0100, Arvin Schnell wrote:
> 
> Hi,
> 
> I realized that the -s/--short option for vgdisplay doesn't work
> in lvm 2.2.00.31 (but also older versions).  That's because the
> option --separator also gets the short option 's' in args.h.
> 
> Attached is a patch.

It the patch ok and will it be included in lvm2?  Or am I missing
something?

ciao Arvin

Re: [linux-lvm] patch for separator option

[Alasdair G Kergon]

> > I realized that the -s/--short option for vgdisplay doesn't work
> > in lvm 2.2.00.31 (but also older versions).  That's because the
> > option --separator also gets the short option 's' in args.h.

> It the patch ok and will it be included in lvm2?
 
Not in that precise form:
Need to check for all other existing conflicts, if any, and
find a generic way to resolve them so 'vgs -s,' for
example can hopefully still work.
[e.g. if multiple possible use of same short form letter with a 
command, the first one listed is the one that applies]

Alasdair
-- 
agk@redhat.com

Re: [linux-lvm] patch for separator option

[Arvin Schnell]

On Sat, Dec 18, 2004 at 08:32:08PM +0000, Alasdair G Kergon wrote:
> > > I realized that the -s/--short option for vgdisplay doesn't work
> > > in lvm 2.2.00.31 (but also older versions).  That's because the
> > > option --separator also gets the short option 's' in args.h.
> 
> > It the patch ok and will it be included in lvm2?
>  
> Not in that precise form:
> Need to check for all other existing conflicts, if any, and
> find a generic way to resolve them so 'vgs -s,' for
> example can hopefully still work.

Checking for conflicts is a good idea.

'-s' is never documented as a shortcut for '--separator', neither
in the help text nor in the man pages.  So 'vgs -s' does not have
to work.  I also don't think it's user-friendly if an option
e.g. '--separator' can be abbreviated by '-s' in only some lvm
tools.

ciao Arvin

Re: [linux-lvm] patch for separator option

[Alasdair G Kergon]

On Mon, Dec 20, 2004 at 12:41:35PM +0100, Arvin Schnell wrote:
> '-s' is never documented as a shortcut for '--separator', neither
> in the help text nor in the man pages.  So 'vgs -s' does not have
> to work.  

Yes it does, because it's a very commonly-used option 
interactively so should have a short form, though the review 
might turn up an alternative letter that's suitable.

Alasdair
-- 
agk@redhat.com