bpo-29958: Minor improvements to zipfile and tarfile CLI. (#944)

4 files changed, 28 insertions, 14 deletions

diff --git a/Lib/tarfile.py b/Lib/tarfile.py
index 2d702dd..9d46e5f 100755
--- a/Lib/tarfile.py
+++ b/Lib/tarfile.py
@@ -2450,11 +2450,11 @@ open = TarFile.open
 def main():
     import argparse
 
-    description = 'A simple command line interface for tarfile module.'
+    description = 'A simple command-line interface for tarfile module.'
     parser = argparse.ArgumentParser(description=description)
     parser.add_argument('-v', '--verbose', action='store_true', default=False,
                         help='Verbose output')
-    group = parser.add_mutually_exclusive_group()
+    group = parser.add_mutually_exclusive_group(required=True)
     group.add_argument('-l', '--list', metavar='<tarfile>',
                        help='Show listing of a tarfile')
     group.add_argument('-e', '--extract', nargs='+',
@@ -2467,7 +2467,7 @@ def main():
                        help='Test if a tarfile is valid')
     args = parser.parse_args()
 
-    if args.test:
+    if args.test is not None:
         src = args.test
         if is_tarfile(src):
             with open(src, 'r') as tar:
@@ -2478,7 +2478,7 @@ def main():
         else:
             parser.exit(1, '{!r} is not a tar archive.\n'.format(src))
 
-    elif args.list:
+    elif args.list is not None:
         src = args.list
         if is_tarfile(src):
             with TarFile.open(src, 'r:*') as tf:
@@ -2486,7 +2486,7 @@ def main():
         else:
             parser.exit(1, '{!r} is not a tar archive.\n'.format(src))
 
-    elif args.extract:
+    elif args.extract is not None:
         if len(args.extract) == 1:
             src = args.extract[0]
             curdir = os.curdir
@@ -2508,7 +2508,7 @@ def main():
         else:
             parser.exit(1, '{!r} is not a tar archive.\n'.format(src))
 
-    elif args.create:
+    elif args.create is not None:
         tar_name = args.create.pop(0)
         _, ext = os.path.splitext(tar_name)
         compressions = {
@@ -2534,8 +2534,5 @@ def main():
         if args.verbose:
             print('{!r} file created.'.format(tar_name))
 
-    else:
-        parser.exit(1, parser.format_help())
-
 if __name__ == '__main__':
     main()
diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py
index 561d5fc..030ace1 100644
--- a/Lib/test/test_tarfile.py
+++ b/Lib/test/test_tarfile.py
@@ -2167,6 +2167,16 @@ class CommandLineTest(unittest.TestCase):
             for tardata in files:
                 tf.add(tardata, arcname=os.path.basename(tardata))
 
+    def test_bad_use(self):
+        rc, out, err = self.tarfilecmd_failure()
+        self.assertEqual(out, b'')
+        self.assertIn(b'usage', err.lower())
+        self.assertIn(b'error', err.lower())
+        self.assertIn(b'required', err.lower())
+        rc, out, err = self.tarfilecmd_failure('-l', '')
+        self.assertEqual(out, b'')
+        self.assertNotEqual(err.strip(), b'')
+
     def test_test_command(self):
         for tar_name in testtarnames:
             for opt in '-t', '--test':
diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py
index 47436e5..46a67d5 100644
--- a/Lib/test/test_zipfile.py
+++ b/Lib/test/test_zipfile.py
@@ -2140,6 +2140,16 @@ class CommandLineTest(unittest.TestCase):
     def zipfilecmd_failure(self, *args):
         return script_helper.assert_python_failure('-m', 'zipfile', *args)
 
+    def test_bad_use(self):
+        rc, out, err = self.zipfilecmd_failure()
+        self.assertEqual(out, b'')
+        self.assertIn(b'usage', err.lower())
+        self.assertIn(b'error', err.lower())
+        self.assertIn(b'required', err.lower())
+        rc, out, err = self.zipfilecmd_failure('-l', '')
+        self.assertEqual(out, b'')
+        self.assertNotEqual(err.strip(), b'')
+
     def test_test_command(self):
         zip_name = findfile('zipdir.zip')
         for opt in '-t', '--test':
diff --git a/Lib/zipfile.py b/Lib/zipfile.py
index 6fdf2c3..550e64f 100644
--- a/Lib/zipfile.py
+++ b/Lib/zipfile.py
@@ -1965,9 +1965,9 @@ class PyZipFile(ZipFile):
 def main(args=None):
     import argparse
 
-    description = 'A simple command line interface for zipfile module.'
+    description = 'A simple command-line interface for zipfile module.'
     parser = argparse.ArgumentParser(description=description)
-    group = parser.add_mutually_exclusive_group()
+    group = parser.add_mutually_exclusive_group(required=True)
     group.add_argument('-l', '--list', metavar='<zipfile>',
                        help='Show listing of a zipfile')
     group.add_argument('-e', '--extract', nargs=2,
@@ -2022,8 +2022,5 @@ def main(args=None):
                     zippath = ''
                 addToZip(zf, path, zippath)
 
-    else:
-        parser.exit(2, parser.format_usage())
-
 if __name__ == "__main__":
     main()