Improve the cli.AskUser() function

This patch improves the AskUser function by allowing it to:
 - choose from multiple choices (instead of only y/n)
 - give help to the user
 - preserve line breaks and whitespace formatting in the message

With this patch, an instance removal looks like this:
root@xen-test1-4:~# gnt-instance remove tsetest1
This will remove the volumes of the instance tsetest1 (including
mirrors), thus removing all the data of the instance. Continue?
y/[n]/?: ?
 y - Perform the operation
 n - Do not perform the operation

This will remove the volumes of the instance tsetest1 (including
mirrors), thus removing all the data of the instance. Continue?
y/[n]/?: y

The patch also removed the _ask_user member of the opts argument, since
external code was using it (and thus it makes no sense for it to be a
private member); now gnt-* scripts are using the AskUser function
directly.

Reviewed-by: ultrotter

diff --git a/lib/cli.py b/lib/cli.py
index 3f42a82..26fa0a0 100644 (file)
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -37,7 +37,7 @@ from optparse import (OptionParser, make_option, TitledHelpFormatter,
                       Option, OptionValueError, SUPPRESS_HELP)
 
 __all__ = ["DEBUG_OPT", "NOHDR_OPT", "SEP_OPT", "GenericMain", "SubmitOpCode",
-           "cli_option", "GenerateTable",
+           "cli_option", "GenerateTable", "AskUser",
            "ARGS_NONE", "ARGS_FIXED", "ARGS_ATLEAST", "ARGS_ANY", "ARGS_ONE",
            "USEUNITS_OPT", "FIELDS_OPT", "FORCE_OPT"]
 
@@ -170,27 +170,59 @@ def _ParseArgs(argv, commands):
   return func, options, args
 
 
-def _AskUser(text):
-  """Ask the user a yes/no question.
+def AskUser(text, choices=None):
+  """Ask the user a question.
 
   Args:
-    questionstring - the question to ask.
+    text - the question to ask.
 
-  Returns:
-    True or False depending on answer (No for False is default).
+    choices - list with elements tuples (input_char, return_value,
+    description); if not given, it will default to: [('y', True,
+    'Perform the operation'), ('n', False, 'Do no do the operation')];
+    note that the '?' char is reserved for help
+
+  Returns: one of the return values from the choices list; if input is
+  not possible (i.e. not running with a tty, we return the last entry
+  from the list
 
   """
+  if choices is None:
+    choices = [('y', True, 'Perform the operation'),
+               ('n', False, 'Do not perform the operation')]
+  if not choices or not isinstance(choices, list):
+    raise errors.ProgrammerError("Invalid choiches argument to AskUser")
+  for entry in choices:
+    if not isinstance(entry, tuple) or len(entry) < 3 or entry[0] == '?':
+      raise errors.ProgrammerError("Invalid choiches element to AskUser")
+
+  answer = choices[-1][1]
+  new_text = []
+  for line in text.splitlines():
+    new_text.append(textwrap.fill(line, 70, replace_whitespace=False))
+  text = "\n".join(new_text)
   try:
     f = file("/dev/tty", "r+")
   except IOError:
-    return False
-  answer = False
+    return answer
   try:
-    f.write(textwrap.fill(text))
-    f.write('\n')
-    f.write("y/[n]: ")
-    line = f.readline(16).strip().lower()
-    answer = line in ('y', 'yes')
+    chars = [entry[0] for entry in choices]
+    chars[-1] = "[%s]" % chars[-1]
+    chars.append('?')
+    maps = dict([(entry[0], entry[1]) for entry in choices])
+    while True:
+      f.write(text)
+      f.write('\n')
+      f.write("/".join(chars))
+      f.write(": ")
+      line = f.readline(2).strip().lower()
+      if line in maps:
+        answer = maps[line]
+        break
+      elif line == '?':
+        for entry in choices:
+          f.write(" %s - %s\n" % (entry[0], entry[2]))
+        f.write("\n")
+        continue
   finally:
     f.close()
   return answer
@@ -231,8 +263,6 @@ def GenericMain(commands):
   if func is None: # parse error
     return 1
 
-  options._ask_user = _AskUser
-
   logger.SetupLogging(debug=options.debug, program=binary)
 
   try:

diff --git a/scripts/gnt-cluster b/scripts/gnt-cluster
index da6bfc7..97ac0e8 100755 (executable)
--- a/scripts/gnt-cluster
+++ b/scripts/gnt-cluster
@@ -78,7 +78,7 @@ def RenameCluster(opts, args):
                 " over the network to the cluster name, the operation is very"
                 " dangerous as the IP address will be removed from the node"
                 " and the change may not go through. Continue?") % name
-    if not opts._ask_user(usertext):
+    if not AskUser(usertext):
       return 1
 
   op = opcodes.OpRenameCluster(name=name)

diff --git a/scripts/gnt-instance b/scripts/gnt-instance
index 243a4b3..f614263 100755 (executable)
--- a/scripts/gnt-instance
+++ b/scripts/gnt-instance
@@ -207,7 +207,7 @@ def ReinstallInstance(opts, args):
   if not opts.force:
     usertext = ("This will reinstall the instance %s and remove "
                 "all data. Continue?") % instance_name
-    if not opts._ask_user(usertext):
+    if not AskUser(usertext):
       return 1
 
   op = opcodes.OpReinstallInstance(instance_name=instance_name,
@@ -232,7 +232,7 @@ def RemoveInstance(opts, args):
     usertext = ("This will remove the volumes of the instance %s"
                 " (including mirrors), thus removing all the data"
                 " of the instance. Continue?") % instance_name
-    if not opts._ask_user(usertext):
+    if not AskUser(usertext):
       return 1
 
   op = opcodes.OpRemoveInstance(instance_name=instance_name)
@@ -394,7 +394,7 @@ def FailoverInstance(opts, args):
     usertext = ("Failover will happen to image %s."
                 " This requires a shutdown of the instance. Continue?" %
                 (instance_name,))
-    if not opts._ask_user(usertext):
+    if not AskUser(usertext):
       return 1
 
   op = opcodes.OpFailoverInstance(instance_name=instance_name,