Merge lp:~parthm/bzr/181124-ls-short-opts into lp:bzr

[Parth Malwankar]

(parthm, #181124) short options for commonly used ls long options.

=== Fixes Bug #181124 ===
Trivial patch to add some short options for 'bzr ls'
I have skipped --from-root as I couldn't think of a good short option. -l as mentioned in the bug report was confusion as 'ls -l' is commonly used.

[Matthew Fuller (fullermd)]

> + Option('null', short_name='Z',

-0 would be more fitting with similar options in oodles of other
programs.

[Parth Malwankar (parthm)]

> > + Option('null', short_name='Z',
>
> -0 would be more fitting with similar options in oodles of other
> programs.

I was actually trying to be consistent with GNU grep but I don't have a specific preference. We can go with -0 if thats preferred.

[John A Meinel (jameinel)]

So our general guidelines were to only give short options to things that actually were used often, and to try to keep them consistent across all commands.

However, since Martin was the one who originally asked for us to be cautious about short options, and he's the one who posted the bug, and said that it should be done, I guess this is ok.

I personally prefer -0 for --null to -Z, but there doesn't seem to be a huge consensus in GNU land. xargs uses -0, find uses -print0, grep uses -Z...

Do we have other tools to include, since so far my sample size has each version happening exactly 1 time... :)

If you're going to be changing this, I would probably pull 'null' out into bzrlib.option, since it is used by cmd_added, and cmd_ignored (IIRC). I would like us to be consistent as much as possible.

Of course, some people have asked for -# w/ bzr.log to mean --last #, which would possibly conflict, but I don't think we can trivially support -# anyway (the option infrastructure would want us to create N options to support N variants of -#, though I'm sure we could do better with some deep work.)

So this is probably "BB:tweak", though I'll mark it needs-fixing as the best approximation.

review: Needs Fixing

[Matthew Fuller (fullermd)]

On Thu, Apr 29, 2010 at 03:36:59PM -0000 I heard the voice of
Parth Malwankar, and lo! it spake thus:
>
> I was actually trying to be consistent with GNU grep but I don't
> have a specific preference. We can go with -0 if thats preferred.

That can vary by the age and divergence of grep.

% man grep
[...]
       --null Output a zero byte (the ASCII NUL character) instead of
[...]
       -Z, --decompress
              Decompress the input data before searching.

% grep -V
grep (GNU grep) 2.5.1-FreeBSD

[Parth Malwankar (parthm)]

> I personally prefer -0 for --null to -Z, but there doesn't seem to be a huge
> consensus in GNU land. xargs uses -0, find uses -print0, grep uses -Z...
>
> Do we have other tools to include, since so far my sample size has each
> version happening exactly 1 time... :)
>

bzr-grep uses -Z. It seems we are leaning towards standardizing -0/--null
for bzr. I can updated bzr-grep for this.

[lp:~parthm/bzr/181124-ls-short-opts]

5197. By Parth Malwankar on 2010-04-30

--null is now a _standard_option

5198. By Parth Malwankar on 2010-04-30

added NEWS entry.

[Parth Malwankar (parthm)]

Thanks for the review comments John, fullermd.

-0/--null is now a _standard_option.
Updated commands ls, modified, and added to used this.
Looks like ignored doesn't take a null option.
Added a NEWS entry to "Improvements" as this is a user visible change.

[John A Meinel (jameinel)]

A very small thing that we don't use:

takes_options = [ 'null' ]

As our formatting. It should just be:

takes_options = ['null']

However, it looks good, and can be cleaned up at merged time.

review: Approve

[lp:~parthm/bzr/181124-ls-short-opts]

5199. By Parth Malwankar on 2010-05-05

cosmetic fix as per coding practice.

5200. By Parth Malwankar on 2010-05-05

merged in changes from trunk and resolved conflict in builtins.py

[Parth Malwankar (parthm)]

> takes_options = ['null']
>

Done. I have also merged in trunk and resolved a conflict in builtins.py due to another mp being landed earlier.

[Parth Malwankar (parthm)]

Thanks Vincent.
I will land this patch.

[lp:~parthm/bzr/181124-ls-short-opts]

5201. By Parth Malwankar on 2010-05-06

merged in changes from trunk.

5202. By Parth Malwankar on 2010-05-07

updated test_help to have --null (which is a standard option now)

5203. By Parth Malwankar on 2010-05-07

merged in changes from trunk.

=== modified file 'NEWS'
--- NEWS	2010-05-05 18:51:16 +0000
+++ NEWS	2010-05-07 04:55:45 +0000
@@ -112,6 +112,10 @@
   (case-sensitive) as false.
   (Brian de Alwis, Vincent Ladeuil)
 
+* ``bzr ls`` now supports short options for existing long options.
+  ``-k/--kind``, ``-i/--ignored``, ``-u/--unknown`` and ``-0/--null``.
+  (Parth Malwankar, #181124)
+
 * ``Config.get_user_option_as_bool`` will now warn if a value cannot
   be interpreted as a boolean.
   (Vincent Ladeuil)

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-05-05 14:11:13 +0000
+++ bzrlib/builtins.py	2010-05-07 04:55:45 +0000
@@ -2032,11 +2032,7 @@
 
     hidden = True
     _see_also = ['status', 'ls']
-    takes_options = ['directory',
-            Option('null',
-                   help='Write an ascii NUL (\\0) separator '
-                   'between files rather than a newline.')
-            ]
+    takes_options = ['directory', 'null']
 
     @display_command
     def run(self, null=False, directory=u'.'):
@@ -2055,11 +2051,7 @@
 
     hidden = True
     _see_also = ['status', 'ls']
-    takes_options = ['directory',
-            Option('null',
-                   help='Write an ascii NUL (\\0) separator '
-                   'between files rather than a newline.')
-            ]
+    takes_options = ['directory', 'null']
 
     @display_command
     def run(self, null=False, directory=u'.'):
@@ -2539,16 +2531,16 @@
                    help='Recurse into subdirectories.'),
             Option('from-root',
                    help='Print paths relative to the root of the branch.'),
-            Option('unknown', help='Print unknown files.'),
+            Option('unknown', short_name='u',
+                help='Print unknown files.'),
             Option('versioned', help='Print versioned files.',
                    short_name='V'),
-            Option('ignored', help='Print ignored files.'),
-            Option('null',
-                   help='Write an ascii NUL (\\0) separator '
-                   'between files rather than a newline.'),
-            Option('kind',
+            Option('ignored', short_name='i',
+                help='Print ignored files.'),
+            Option('kind', short_name='k',
                    help='List entries of a particular kind: file, directory, symlink.',
                    type=unicode),
+            'null',
             'show-ids',
             'directory',
             ]

=== modified file 'bzrlib/option.py'
--- bzrlib/option.py	2010-05-03 09:19:15 +0000
+++ bzrlib/option.py	2010-05-07 04:55:45 +0000
@@ -506,6 +506,9 @@
 # Declare the standard options
 _standard_option('help', short_name='h',
                  help='Show help message.')
+_standard_option('null', short_name='0',
+                 help='Use an ASCII NUL (\\0) separator rather than '
+                      'a newline.')
 _standard_option('usage',
                  help='Show usage message and options.')
 _standard_option('verbose', short_name='v',

=== modified file 'bzrlib/tests/test_help.py'
--- bzrlib/tests/test_help.py	2010-04-23 08:51:52 +0000
+++ bzrlib/tests/test_help.py	2010-05-07 04:55:45 +0000
@@ -122,6 +122,7 @@
             '\n'
             'Options:\n'
             '  --usage        Show usage message and options.\n'
+            '  -0, --null     Use an ASCII NUL (\\0) separator rather than a newline.\n'
             '  -v, --verbose  Display more information.\n'
             '  -q, --quiet    Only display errors and warnings.\n'
             '  -h, --help     Show help message.\n'
@@ -142,6 +143,7 @@
             '\n'
             ':Options:\n'
             '  --usage        Show usage message and options.\n'
+            '  -0, --null     Use an ASCII NUL (\\0) separator rather than a newline.\n'
             '  -v, --verbose  Display more information.\n'
             '  -q, --quiet    Only display errors and warnings.\n'
             '  -h, --help     Show help message.\n'
@@ -177,6 +179,7 @@
             '\n'
             'Options:\n'
             '  --usage        Show usage message and options.\n'
+            '  -0, --null     Use an ASCII NUL (\\0) separator rather than a newline.\n'
             '  -v, --verbose  Display more information.\n'
             '  -q, --quiet    Only display errors and warnings.\n'
             '  -h, --help     Show help message.\n'
@@ -196,6 +199,7 @@
             '\n'
             'Options:\n'
             '  --usage        Show usage message and options.\n'
+            '  -0, --null     Use an ASCII NUL (\\0) separator rather than a newline.\n'
             '  -v, --verbose  Display more information.\n'
             '  -q, --quiet    Only display errors and warnings.\n'
             '  -h, --help     Show help message.\n'
@@ -230,6 +234,7 @@
             '\n'
             'Options:\n'
             '  --usage        Show usage message and options.\n'
+            '  -0, --null     Use an ASCII NUL (\\0) separator rather than a newline.\n'
             '  -v, --verbose  Display more information.\n'
             '  -q, --quiet    Only display errors and warnings.\n'
             '  -h, --help     Show help message.\n'
@@ -273,6 +278,7 @@
             '\n'
             'Options:\n'
             '  --usage        Show usage message and options.\n'
+            '  -0, --null     Use an ASCII NUL (\\0) separator rather than a newline.\n'
             '  -v, --verbose  Display more information.\n'
             '  -q, --quiet    Only display errors and warnings.\n'
             '  -h, --help     Show help message.\n'