From f181a0458ac65dc9b658c3330caa49a8b0cdc500 Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 1 Aug 2008 11:46:24 -0700
Subject: [PATCH] CLI: simplify argument-passing, use options= for everthing,
 including stdout

---
 src/allmydata/scripts/cli.py             | 111 ++++++-----------------
 src/allmydata/scripts/common.py          |   5 +
 src/allmydata/scripts/runner.py          |  12 ++-
 src/allmydata/scripts/tahoe_add_alias.py |  12 ++-
 src/allmydata/scripts/tahoe_cp.py        |  41 +++++----
 src/allmydata/scripts/tahoe_get.py       |   9 +-
 src/allmydata/scripts/tahoe_ls.py        |  18 ++--
 src/allmydata/scripts/tahoe_mkdir.py     |   7 +-
 src/allmydata/scripts/tahoe_mv.py        |   9 +-
 src/allmydata/scripts/tahoe_put.py       |  16 +++-
 src/allmydata/scripts/tahoe_rm.py        |  12 ++-
 src/allmydata/test/test_system.py        |  10 +-
 12 files changed, 140 insertions(+), 122 deletions(-)

diff --git a/src/allmydata/scripts/cli.py b/src/allmydata/scripts/cli.py
index ccbb78be..b76d30dc 100644
--- a/src/allmydata/scripts/cli.py
+++ b/src/allmydata/scripts/cli.py
@@ -204,131 +204,78 @@ subCommands = [
     ["repl", None, ReplOptions, "Open a python interpreter"],
     ]
 
-def mkdir(config, stdout, stderr):
+def mkdir(options):
     from allmydata.scripts import tahoe_mkdir
-    rc = tahoe_mkdir.mkdir(config['node-url'],
-                           config.aliases,
-                           config.where,
-                           stdout, stderr)
+    rc = tahoe_mkdir.mkdir(options)
     return rc
 
-def add_alias(config, stdout, stderr):
+def add_alias(options):
     from allmydata.scripts import tahoe_add_alias
-    rc = tahoe_add_alias.add_alias(config['node-directory'],
-                                   config.alias,
-                                   config.cap,
-                                   stdout, stderr)
+    rc = tahoe_add_alias.add_alias(options)
     return rc
 
-def list_aliases(config, stdout, stderr):
+def list_aliases(options):
     from allmydata.scripts import tahoe_add_alias
-    rc = tahoe_add_alias.list_aliases(config['node-directory'],
-                                      stdout, stderr)
+    rc = tahoe_add_alias.list_aliases(options)
     return rc
 
-def list(config, stdout, stderr):
+def list(options):
     from allmydata.scripts import tahoe_ls
-    rc = tahoe_ls.list(config['node-url'],
-                       config.aliases,
-                       config.where,
-                       config,
-                       stdout, stderr)
+    rc = tahoe_ls.list(options)
     return rc
 
-def get(config, stdout, stderr):
+def get(options):
     from allmydata.scripts import tahoe_get
-    rc = tahoe_get.get(config['node-url'],
-                       config.aliases,
-                       config.from_file,
-                       config.to_file,
-                       stdout, stderr)
+    rc = tahoe_get.get(options)
     if rc == 0:
-        if config.to_file is None:
+        if options.to_file is None:
             # be quiet, since the file being written to stdout should be
             # proof enough that it worked, unless the user is unlucky
             # enough to have picked an empty file
             pass
         else:
-            print >>stderr, "%s retrieved and written to %s" % \
-                  (config.from_file, config.to_file)
+            print >>options.stderr, "%s retrieved and written to %s" % \
+                  (options.from_file, options.to_file)
     return rc
 
-def put(config, stdout, stderr, stdin=sys.stdin):
+def put(options):
     from allmydata.scripts import tahoe_put
-    if config['quiet']:
-        verbosity = 0
-    else:
-        verbosity = 2
-    rc = tahoe_put.put(config['node-url'],
-                       config.aliases,
-                       config.from_file,
-                       config.to_file,
-                       config['mutable'],
-                       verbosity,
-                       stdin, stdout, stderr)
+    rc = tahoe_put.put(options)
     return rc
 
-def cp(config, stdout, stderr):
+def cp(options):
     from allmydata.scripts import tahoe_cp
-    if config['quiet']:
-        verbosity = 0
-    else:
-        verbosity = 2
-    rc = tahoe_cp.copy(config['node-url'],
-                       config,
-                       config.aliases,
-                       config.sources,
-                       config.destination,
-                       verbosity,
-                       stdout, stderr)
+    rc = tahoe_cp.copy(options)
     return rc
 
-def rm(config, stdout, stderr):
+def rm(options):
     from allmydata.scripts import tahoe_rm
-    if config['quiet']:
-        verbosity = 0
-    else:
-        verbosity = 2
-    rc = tahoe_rm.rm(config['node-url'],
-                     config.aliases,
-                     config.where,
-                     verbosity,
-                     stdout, stderr)
+    rc = tahoe_rm.rm(options)
     return rc
 
-def mv(config, stdout, stderr):
+def mv(options):
     from allmydata.scripts import tahoe_mv
-    rc = tahoe_mv.mv(config['node-url'],
-                     config.aliases,
-                     config.from_file,
-                     config.to_file,
-                     stdout, stderr,
-                     mode="move")
+    rc = tahoe_mv.mv(options, mode="move")
     return rc
 
-def ln(config, stdout, stderr):
+def ln(options):
     from allmydata.scripts import tahoe_mv
-    rc = tahoe_mv.mv(config['node-url'],
-                     config.aliases,
-                     config.from_file,
-                     config.to_file,
-                     stdout, stderr,
-                     mode="link")
+    rc = tahoe_mv.mv(options, mode="link")
     return rc
 
-def webopen(config, stdout, stderr):
+def webopen(options):
     import urllib, webbrowser
-    nodeurl = config['node-url']
+    nodeurl = options['node-url']
     if nodeurl[-1] != "/":
         nodeurl += "/"
-    root_cap = config.aliases["tahoe"]
+    root_cap = options.aliases["tahoe"]
     url = nodeurl + "uri/%s/" % urllib.quote(root_cap)
-    if config['vdrive_pathname']:
-        url += urllib.quote(config['vdrive_pathname'])
+    if options['vdrive_pathname']:
+        url += urllib.quote(options['vdrive_pathname'])
     webbrowser.open(url)
     return 0
 
-def repl(config, stdout, stderr):
+def repl(options):
     import code
     return code.interact()
 
diff --git a/src/allmydata/scripts/common.py b/src/allmydata/scripts/common.py
index 2fa14617..41c1fccf 100644
--- a/src/allmydata/scripts/common.py
+++ b/src/allmydata/scripts/common.py
@@ -4,6 +4,11 @@ from twisted.python import usage
 
 
 class BaseOptions:
+    # unit tests can override these to point at StringIO instances
+    stdin = sys.stdin
+    stdout = sys.stdout
+    stderr = sys.stderr
+
     optFlags = [
         ["quiet", "q", "Operate silently."],
         ["version", "V", "Display version numbers and exit."],
diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py
index eb8bbae8..1ac2652c 100644
--- a/src/allmydata/scripts/runner.py
+++ b/src/allmydata/scripts/runner.py
@@ -23,8 +23,10 @@ class Options(BaseOptions, usage.Options):
         if not hasattr(self, 'subOptions'):
             raise usage.UsageError("must specify a command")
 
-def runner(argv, run_by_human=True, stdout=sys.stdout, stderr=sys.stderr,
-                 install_node_control=True, additional_commands=None):
+def runner(argv,
+           run_by_human=True,
+           stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr,
+           install_node_control=True, additional_commands=None):
 
     config = Options()
     if install_node_control:
@@ -53,6 +55,10 @@ def runner(argv, run_by_human=True, stdout=sys.stdout, stderr=sys.stderr,
     if config['quiet']:
         stdout = StringIO()
 
+    so.stdout = stdout
+    so.stderr = stderr
+    so.stdin = stdin
+
     rc = 0
     if command in create_node.dispatch:
         for basedir in so.basedirs:
@@ -63,7 +69,7 @@ def runner(argv, run_by_human=True, stdout=sys.stdout, stderr=sys.stderr,
     elif command in debug.dispatch:
         rc = debug.dispatch[command](so, stdout, stderr)
     elif command in cli.dispatch:
-        rc = cli.dispatch[command](so, stdout, stderr)
+        rc = cli.dispatch[command](so)
     elif command in keygen.dispatch:
         rc = keygen.dispatch[command](so, stdout, stderr)
     elif command in ac_dispatch:
diff --git a/src/allmydata/scripts/tahoe_add_alias.py b/src/allmydata/scripts/tahoe_add_alias.py
index c526b853..162faf55 100644
--- a/src/allmydata/scripts/tahoe_add_alias.py
+++ b/src/allmydata/scripts/tahoe_add_alias.py
@@ -3,7 +3,12 @@ import os.path
 from allmydata import uri
 from allmydata.scripts.common import get_aliases
 
-def add_alias(nodedir, alias, cap, stdout, stderr):
+def add_alias(options):
+    nodedir = options['node-directory']
+    alias = options.alias
+    cap = options.cap
+    stdout = options.stdout
+    stderr = options.stderr
     aliasfile = os.path.join(nodedir, "private", "aliases")
     cap = uri.from_string_dirnode(cap).to_string()
     assert ":" not in alias
@@ -15,7 +20,10 @@ def add_alias(nodedir, alias, cap, stdout, stderr):
     print >>stdout, "Alias '%s' added" % (alias,)
     return 0
 
-def list_aliases(nodedir, stdout, stderr):
+def list_aliases(options):
+    nodedir = options['node-directory']
+    stdout = options.stdout
+    stderr = options.stderr
     aliases = get_aliases(nodedir)
     alias_names = sorted(aliases.keys())
     max_width = max([len(name) for name in alias_names] + [0])
diff --git a/src/allmydata/scripts/tahoe_cp.py b/src/allmydata/scripts/tahoe_cp.py
index 73d5c94b..32e0ca9e 100644
--- a/src/allmydata/scripts/tahoe_cp.py
+++ b/src/allmydata/scripts/tahoe_cp.py
@@ -397,29 +397,31 @@ class TahoeDirectoryTarget:
         POST(url, body)
 
 class Copier:
-    def __init__(self, nodeurl, config, aliases,
-                 verbosity, stdout, stderr,
-                 progressfunc=None):
+
+    def do_copy(self, options, progressfunc=None):
+        if options['quiet']:
+            verbosity = 0
+        else:
+            verbosity = 2
+
+        nodeurl = options['node-url']
         if nodeurl[-1] != "/":
             nodeurl += "/"
         self.nodeurl = nodeurl
         self.progressfunc = progressfunc
-        self.config = config
-        self.aliases = aliases
+        self.options = options
+        self.aliases = options.aliases
         self.verbosity = verbosity
-        if config["verbose"] and not self.progressfunc:
+        self.stdout = options.stdout
+        self.stderr = options.stderr
+        if options["verbose"] and not self.progressfunc:
             def progress(message):
-                print >>stderr, message
+                print >>self.stderr, message
             self.progressfunc = progress
-        self.stdout = stdout
-        self.stderr = stderr
         self.cache = {}
-
-    def to_stderr(self, text):
-        print >>self.stderr, text
-
-    def do_copy(self, source_specs, destination_spec):
-        recursive = self.config["recursive"]
+        source_specs = options.sources
+        destination_spec = options.destination
+        recursive = self.options["recursive"]
 
         target = self.get_target_info(destination_spec)
 
@@ -470,6 +472,9 @@ class Copier:
         self.to_stderr("unknown target")
         return 1
 
+    def to_stderr(self, text):
+        print >>self.stderr, text
+
     def get_target_info(self, destination_spec):
         rootcap, path = get_alias(self.aliases, destination_spec, None)
         if rootcap == DefaultAliasMarker:
@@ -705,7 +710,5 @@ class Copier:
         return graphs
 
 
-def copy(nodeurl, config, aliases, sources, destination,
-         verbosity, stdout, stderr):
-    c = Copier(nodeurl, config, aliases, verbosity, stdout, stderr)
-    return c.do_copy(sources, destination)
+def copy(options):
+    return Copier().do_copy(options)
diff --git a/src/allmydata/scripts/tahoe_get.py b/src/allmydata/scripts/tahoe_get.py
index 09b70bb2..927b210e 100644
--- a/src/allmydata/scripts/tahoe_get.py
+++ b/src/allmydata/scripts/tahoe_get.py
@@ -3,7 +3,14 @@ import urllib
 from allmydata.scripts.common import get_alias, DEFAULT_ALIAS, escape_path
 from allmydata.scripts.common_http import do_http
 
-def get(nodeurl, aliases, from_file, to_file, stdout, stderr):
+def get(options):
+    nodeurl = options['node-url']
+    aliases = options.aliases
+    from_file = options.from_file
+    to_file = options.to_file
+    stdout = options.stdout
+    stderr = options.stderr
+
     if nodeurl[-1] != "/":
         nodeurl += "/"
     rootcap, path = get_alias(aliases, from_file, DEFAULT_ALIAS)
diff --git a/src/allmydata/scripts/tahoe_ls.py b/src/allmydata/scripts/tahoe_ls.py
index 731cc527..39d6d49b 100644
--- a/src/allmydata/scripts/tahoe_ls.py
+++ b/src/allmydata/scripts/tahoe_ls.py
@@ -4,7 +4,13 @@ import simplejson
 from allmydata.scripts.common import get_alias, DEFAULT_ALIAS, escape_path
 from allmydata.scripts.common_http import do_http
 
-def list(nodeurl, aliases, where, config, stdout, stderr):
+def list(options):
+    nodeurl = options['node-url']
+    aliases = options.aliases
+    where = options.where
+    stdout = options.stdout
+    stderr = options.stderr
+
     if not nodeurl.endswith("/"):
         nodeurl += "/"
     if where.endswith("/"):
@@ -26,7 +32,7 @@ def list(nodeurl, aliases, where, config, stdout, stderr):
                                                         resp.read())
     data = resp.read()
 
-    if config['json']:
+    if options['json']:
         print >>stdout, data
         return
 
@@ -90,16 +96,16 @@ def list(nodeurl, aliases, where, config, stdout, stderr):
         uri = rw_uri or ro_uri
 
         line = []
-        if config["long"]:
+        if options["long"]:
             line.append(t0+t1+t2+t3)
             line.append(size)
             line.append(ctime_s)
-        if not config["classify"]:
+        if not options["classify"]:
             classify = ""
         line.append(name + classify)
-        if config["uri"]:
+        if options["uri"]:
             line.append(uri)
-        if config["readonly-uri"]:
+        if options["readonly-uri"]:
             line.append(ro_uri or "-")
 
         rows.append(line)
diff --git a/src/allmydata/scripts/tahoe_mkdir.py b/src/allmydata/scripts/tahoe_mkdir.py
index 41c0a4f4..1c6f31a6 100644
--- a/src/allmydata/scripts/tahoe_mkdir.py
+++ b/src/allmydata/scripts/tahoe_mkdir.py
@@ -3,7 +3,12 @@ import urllib
 from allmydata.scripts.common_http import do_http, check_http_error
 from allmydata.scripts.common import get_alias, DEFAULT_ALIAS
 
-def mkdir(nodeurl, aliases, where, stdout, stderr):
+def mkdir(options):
+    nodeurl = options['node-url']
+    aliases = options.aliases
+    where = options.where
+    stdout = options.stdout
+    stderr = options.stderr
     if not nodeurl.endswith("/"):
         nodeurl += "/"
     if where:
diff --git a/src/allmydata/scripts/tahoe_mv.py b/src/allmydata/scripts/tahoe_mv.py
index 41bb5fd9..c06631f9 100644
--- a/src/allmydata/scripts/tahoe_mv.py
+++ b/src/allmydata/scripts/tahoe_mv.py
@@ -7,7 +7,14 @@ from allmydata.scripts.common_http import do_http
 
 # this script is used for both 'mv' and 'ln'
 
-def mv(nodeurl, aliases, from_file, to_file, stdout, stderr, mode="move"):
+def mv(options, mode="move"):
+    nodeurl = options['node-url']
+    aliases = options.aliases
+    from_file = options.from_file
+    to_file = options.to_file
+    stdout = options.stdout
+    stderr = options.stderr
+
     if nodeurl[-1] != "/":
         nodeurl += "/"
     rootcap, path = get_alias(aliases, from_file, DEFAULT_ALIAS)
diff --git a/src/allmydata/scripts/tahoe_put.py b/src/allmydata/scripts/tahoe_put.py
index 6d87b5e5..25787693 100644
--- a/src/allmydata/scripts/tahoe_put.py
+++ b/src/allmydata/scripts/tahoe_put.py
@@ -4,13 +4,25 @@ import urllib
 from allmydata.scripts.common_http import do_http
 from allmydata.scripts.common import get_alias, DEFAULT_ALIAS, escape_path
 
-def put(nodeurl, aliases, from_file, to_file, mutable,
-        verbosity, stdin, stdout, stderr):
+def put(options):
     """
     @param verbosity: 0, 1, or 2, meaning quiet, verbose, or very verbose
 
     @return: a Deferred which eventually fires with the exit code
     """
+    nodeurl = options['node-url']
+    aliases = options.aliases
+    from_file = options.from_file
+    to_file = options.to_file
+    mutable = options['mutable']
+    if options['quiet']:
+        verbosity = 0
+    else:
+        verbosity = 2
+    stdin = options.stdin
+    stdout = options.stdout
+    stderr = options.stderr
+
     if nodeurl[-1] != "/":
         nodeurl += "/"
     if to_file:
diff --git a/src/allmydata/scripts/tahoe_rm.py b/src/allmydata/scripts/tahoe_rm.py
index 2ef36975..a0ffe719 100644
--- a/src/allmydata/scripts/tahoe_rm.py
+++ b/src/allmydata/scripts/tahoe_rm.py
@@ -3,12 +3,22 @@ import urllib
 from allmydata.scripts.common_http import do_http
 from allmydata.scripts.common import get_alias, DEFAULT_ALIAS, escape_path
 
-def rm(nodeurl, aliases, where, verbosity, stdout, stderr):
+def rm(options):
     """
     @param verbosity: 0, 1, or 2, meaning quiet, verbose, or very verbose
 
     @return: a Deferred which eventually fires with the exit code
     """
+    nodeurl = options['node-url']
+    aliases = options.aliases
+    where = options.where
+    if options['quiet']:
+        verbosity = 0
+    else:
+        verbosity = 2
+    stdout = options.stdout
+    stderr = options.stderr
+
     if nodeurl[-1] != "/":
         nodeurl += "/"
     rootcap, path = get_alias(aliases, where, DEFAULT_ALIAS)
diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py
index d26c8b16..ed76d6c8 100644
--- a/src/allmydata/test/test_system.py
+++ b/src/allmydata/test/test_system.py
@@ -1344,9 +1344,9 @@ class SystemTest(SystemTestMixin, unittest.TestCase):
         def _check_ls((out,err), expected_children, unexpected_children=[]):
             self.failUnlessEqual(err, "")
             for s in expected_children:
-                self.failUnless(s in out, s)
+                self.failUnless(s in out, (s,out))
             for s in unexpected_children:
-                self.failIf(s in out, s)
+                self.failIf(s in out, (s,out))
 
         def _check_ls_root((out,err)):
             self.failUnless("personal" in out)
@@ -1442,8 +1442,10 @@ class SystemTest(SystemTestMixin, unittest.TestCase):
             o.parseOptions(args)
             stdin = StringIO(data)
             stdout, stderr = StringIO(), StringIO()
-            d = threads.deferToThread(cli.put, o,
-                                      stdout=stdout, stderr=stderr, stdin=stdin)
+            o.stdin = stdin
+            o.stdout = stdout
+            o.stderr = stderr
+            d = threads.deferToThread(cli.put, o)
             def _done(res):
                 return stdout.getvalue(), stderr.getvalue()
             d.addCallback(_done)
-- 
2.45.2