Mon Jul 21 23:01:38 2008 Google Inc. <opensource@google.com>

	* google-gflags: version 0.9
	* Add the ability to validate a command-line flag (csilvers)
	* Add completion support for commandline flags in bash (daven)
	* Add -W compile flags to Makefile, when using gcc (csilvers)
	* Allow helpstring to be NULL (cristianoc)
	* Improved documentation of classes in the .cc file (csilvers)
	* Fix python bug with AppendFlagValues + shortnames (jjtswan)
	* Use bool instead of int for boolean flags in gflags.py (bcmills)
	* Simplify the way we declare flags, now more foolproof (csilvers)
	* Better error messages when bool flags collide (colohan)
	* Only evaluate DEFINE_foo macro args once (csilvers)


git-svn-id: https://gflags.googlecode.com/svn/trunk@23 6586e3c6-dcc4-952a-343f-ff74eb82781d

ChangeLog

+Mon Jul 21 23:01:38 2008  Google Inc. <opensource@google.com>
+
+	* google-gflags: version 0.9
+	* Add the ability to validate a command-line flag (csilvers)
+	* Add completion support for commandline flags in bash (daven)
+	* Add -W compile flags to Makefile, when using gcc (csilvers)
+	* Allow helpstring to be NULL (cristianoc)
+	* Improved documentation of classes in the .cc file (csilvers)
+	* Fix python bug with AppendFlagValues + shortnames (jjtswan)
+	* Use bool instead of int for boolean flags in gflags.py (bcmills)
+	* Simplify the way we declare flags, now more foolproof (csilvers)
+	* Better error messages when bool flags collide (colohan)
+	* Only evaluate DEFINE_foo macro args once (csilvers)
+
 Wed Mar 26 15:20:18 2008  Google Inc. <opensource@google.com>
 
 	* google-gflags: version 0.8

Makefile.am

@@ -11,10 +11,20 @@ ACLOCAL_AMFLAGS = -I m4
 # This is so we can #include <google/foo>
 AM_CPPFLAGS = -I$(top_srcdir)/src
 
+# This is mostly based on configure options
+AM_CXXFLAGS =
+
+# These are good warnings to turn on by default,
+if GCC
+AM_CXXFLAGS += -Wall -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare
+endif
+
 googleincludedir = $(includedir)/google
 ## The .h files you want to install (that is, .h files that people
 ## who install this package can include in their own applications.)
-googleinclude_HEADERS = src/google/gflags.h
+googleinclude_HEADERS = src/google/gflags.h src/google/gflags_completions.h
+
+bin_SCRIPTS = src/gflags_completions.sh
 
 docdir = $(prefix)/share/doc/$(PACKAGE)-$(VERSION)
 ## This is for HTML and other documentation you want to install.
@@ -42,9 +52,10 @@ CLEANFILES =
 
 lib_LTLIBRARIES += libgflags.la
 libgflags_la_SOURCES = $(googleinclude_HEADERS) src/config.h \
-                       src/gflags.cc src/gflags_reporting.cc
-libgflags_la_CXXFLAGS = $(PTRHEAD_CFLAGS) -DNDEBUG
-libgflags_la_LDFLAGS = $(PTRHEAD_CFLAGS)
+                       src/gflags.cc src/gflags_reporting.cc \
+                       src/gflags_completions.cc
+libgflags_la_CXXFLAGS = $(PTHREAD_CFLAGS) -DNDEBUG
+libgflags_la_LDFLAGS = $(PTHREAD_CFLAGS)
 libgflags_la_LIBADD = $(PTHREAD_LIBS)
 
 TESTS += gflags_unittest

configure.ac

@@ -4,7 +4,7 @@
 # make sure we're interpreted by some minimal autoconf
 AC_PREREQ(2.57)
 
-AC_INIT(gflags, 0.8, opensource@google.com)
+AC_INIT(gflags, 0.9, opensource@google.com)
 # The argument here is just something that should be in the current directory
 # (for sanity checking)
 AC_CONFIG_SRCDIR(README)
@@ -15,6 +15,7 @@ AM_CONFIG_HEADER(src/config.h)
 AC_PROG_CC
 AC_PROG_CPP
 AC_PROG_CXX
+AM_CONDITIONAL(GCC, test "$GCC" = yes)   # let the Makefile know if we're gcc
 
 # Uncomment this if you'll be exporting libraries (.so's)
 AC_PROG_LIBTOOL
@@ -67,5 +68,5 @@ AC_SUBST(ac_cv_have___uint16)
 ## Check out ../autoconf/ for other macros you can call to do useful stuff
 
 # Write generated configuration file, and also .h files
-AC_CONFIG_FILES([Makefile src/google/gflags.h])
+AC_CONFIG_FILES([Makefile src/google/gflags.h src/google/gflags_completions.h])
 AC_OUTPUT

doc/gflags.html

@@ -189,6 +189,38 @@ file.
 <code>#include</code> will make explicit the dependency between the
 two files. This causes the flag to be a global variable.</p>
 
+
+<h2> <A name=validate>RegisterFlagValidator: Sanity-checking Flag Values</A> </h2>
+
+<p>After DEFINE-ing a flag, you may optionally register a validator
+function with the flag.  If you do this, after the flag is parsed from
+the commandline, and whenever its value is changes via a call to
+<code>SetCommandLineOption()</code>, the validator function is called
+with the new value as an argument.  The validator function should
+return 'true' if the flag value is valid, and false otherwise. 
+
+<p>Here is an example use of this functionality:</p>
+<pre>
+static bool ValidatePort(const char* flagname, int32 value) {
+   if (value > 0 && value < 32768)   // value is ok
+     return true;
+   printf("Invalid value for --%s: %d\n", flagname, (int)value);
+   return false;
+}
+DEFINE_int32(port, 0, "What port to listen on");
+static const bool port_dummy = RegisterFlagValidator(&FLAGS_port, &ValidatePort);
+</pre>
+
+<p>By doing the registration at global initialization time (right
+after the DEFINE), we ensure that the registration happens before
+the commandline is parsed at the beginning of <code>main()</code>.</p>
+
+<p><code>RegisterFlagValidator()</code> returns true if the
+registration is successful.  It return false if the registration fails
+because a) the first argument does not refer to a commandline flag, or
+b) a different validator has already been registered for this flag.</p>
+
+
 <h2> <A name=together>Putting It Together: How to Set Up Flags</A> </h2>
 
 <p>The final piece is the one that tells the executable to process the

m4/acx_pthread.m4

 # This was retrieved from
-#    http://0pointer.de/cgi-bin/viewcvs.cgi/trunk/common/acx_pthread.m4?rev=1227
+#    http://svn.0pointer.de/viewvc/trunk/common/acx_pthread.m4?revision=1277&root=avahi
 # See also (perhaps for new versions?)
-#    http://0pointer.de/cgi-bin/viewcvs.cgi/trunk/common/acx_pthread.m4
+#    http://svn.0pointer.de/viewvc/trunk/common/acx_pthread.m4?root=avahi
+#
+# We've rewritten the inconsistency check code (from avahi), to work
+# more broadly.  In particular, it no longer assumes ld accepts -zdefs.
+# This caused a restructing of the code, but the functionality has only
+# changed a little.
 
 dnl @synopsis ACX_PTHREAD([ACTION-IF-FOUND[, ACTION-IF-NOT-FOUND]])
 dnl
@@ -231,108 +236,113 @@ if test "x$acx_pthread_ok" = xyes; then
           PTHREAD_CC=$CC
 	fi
 
-   # The next part tries to detect GCC inconsistency with -shared on some
-   # architectures and systems. The problem is that in certain
-   # configurations, when -shared is specified, GCC "forgets" to
-   # internally use various flags which are still necessary.
-   
-   AC_MSG_CHECKING([whether to check for GCC pthread/shared inconsistencies])
-   check_inconsistencies=yes
-   case "${host_cpu}-${host_os}" in
-     *-darwin*) check_inconsistencies=no ;;
-   esac
-   if test x"$GCC" != xyes -o "x$check_inconsistencies" != xyes ; then
-      AC_MSG_RESULT([no])
-   else
-      AC_MSG_RESULT([yes])
-
-      # In order not to create several levels of indentation, we test
-      # the value of "$ok" until we find out the cure or run out of
-      # ideas.
-      ok="no"
-
-      #
-      # Prepare the flags
-      #
-      save_CFLAGS="$CFLAGS"
-      save_LIBS="$LIBS"
-      save_CC="$CC"
-      # Try with the flags determined by the earlier checks.
-      #
-      # -Wl,-z,defs forces link-time symbol resolution, so that the
-      # linking checks with -shared actually have any value
-      #
-      # FIXME: -fPIC is required for -shared on many architectures,
-      # so we specify it here, but the right way would probably be to
-      # properly detect whether it is actually required.
-      CFLAGS="-shared -fPIC -Wl,-z,defs $CFLAGS $PTHREAD_CFLAGS"
-      LIBS="$PTHREAD_LIBS $LIBS"
-      CC="$PTHREAD_CC"
-
-      AC_MSG_CHECKING([whether -pthread is sufficient with -shared])
-      AC_TRY_LINK([#include <pthread.h>],
-         [pthread_t th; pthread_join(th, 0);
-         pthread_attr_init(0); pthread_cleanup_push(0, 0);
-         pthread_create(0,0,0,0); pthread_cleanup_pop(0); ],
-         [ok=yes])
-      
-      if test "x$ok" = xyes; then
-         AC_MSG_RESULT([yes])
-      else
-         AC_MSG_RESULT([no])
-      fi
-   
-      #
-      # Linux gcc on some architectures such as mips/mipsel forgets
-      # about -lpthread
-      #
-      if test x"$ok" = xno; then
-         AC_MSG_CHECKING([whether -lpthread fixes that])
-         LIBS="-lpthread $PTHREAD_LIBS $save_LIBS"
-         AC_TRY_LINK([#include <pthread.h>],
-            [pthread_t th; pthread_join(th, 0);
-            pthread_attr_init(0); pthread_cleanup_push(0, 0);
-            pthread_create(0,0,0,0); pthread_cleanup_pop(0); ],
-            [ok=yes])
-   
-         if test "x$ok" = xyes; then
-            AC_MSG_RESULT([yes])
-            PTHREAD_LIBS="-lpthread $PTHREAD_LIBS"
-         else
-            AC_MSG_RESULT([no])
-         fi
-      fi
-      #
-      # FreeBSD 4.10 gcc forgets to use -lc_r instead of -lc
-      #
-      if test x"$ok" = xno; then
-         AC_MSG_CHECKING([whether -lc_r fixes that])
-         LIBS="-lc_r $PTHREAD_LIBS $save_LIBS"
-         AC_TRY_LINK([#include <pthread.h>],
-             [pthread_t th; pthread_join(th, 0);
-              pthread_attr_init(0); pthread_cleanup_push(0, 0);
-              pthread_create(0,0,0,0); pthread_cleanup_pop(0); ],
-             [ok=yes])
-   
-         if test "x$ok" = xyes; then
-            AC_MSG_RESULT([yes])
-            PTHREAD_LIBS="-lc_r $PTHREAD_LIBS"
-         else
-            AC_MSG_RESULT([no])
-         fi
-      fi
-      if test x"$ok" = xno; then
-         # OK, we have run out of ideas
-         AC_MSG_WARN([Impossible to determine how to use pthreads with shared libraries])
-
-         # so it's not safe to assume that we may use pthreads
-         acx_pthread_ok=no
-      fi
-
-      CFLAGS="$save_CFLAGS"
-      LIBS="$save_LIBS"
-      CC="$save_CC"
-   fi
+	# The next part tries to detect GCC inconsistency with -shared on some
+	# architectures and systems. The problem is that in certain
+	# configurations, when -shared is specified, GCC "forgets" to
+	# internally use various flags which are still necessary.
+	
+	#
+	# Prepare the flags
+	#
+	save_CFLAGS="$CFLAGS"
+	save_LIBS="$LIBS"
+	save_CC="$CC"
+	
+	# Try with the flags determined by the earlier checks.
+	#
+	# -Wl,-z,defs forces link-time symbol resolution, so that the
+	# linking checks with -shared actually have any value
+	#
+	# FIXME: -fPIC is required for -shared on many architectures,
+	# so we specify it here, but the right way would probably be to
+	# properly detect whether it is actually required.
+	CFLAGS="-shared -fPIC -Wl,-z,defs $CFLAGS $PTHREAD_CFLAGS"
+	LIBS="$PTHREAD_LIBS $LIBS"
+	CC="$PTHREAD_CC"
+	
+	# In order not to create several levels of indentation, we test
+	# the value of "$done" until we find the cure or run out of ideas.
+	done="no"
+	
+	# First, make sure the CFLAGS we added are actually accepted by our
+	# compiler.  If not (and OS X's ld, for instance, does not accept -z),
+	# then we can't do this test.
+	if test x"$done" = xno; then
+	   AC_MSG_CHECKING([whether to check for GCC pthread/shared inconsistencies])
+	   AC_TRY_LINK(,, , [done=yes])
+	
+	   if test "x$done" = xyes ; then
+	      AC_MSG_RESULT([no])
+	   else
+	      AC_MSG_RESULT([yes])
+	   fi
+	fi
+	
+	if test x"$done" = xno; then
+	   AC_MSG_CHECKING([whether -pthread is sufficient with -shared])
+	   AC_TRY_LINK([#include <pthread.h>],
+	      [pthread_t th; pthread_join(th, 0);
+	      pthread_attr_init(0); pthread_cleanup_push(0, 0);
+	      pthread_create(0,0,0,0); pthread_cleanup_pop(0); ],
+	      [done=yes])
+	   
+	   if test "x$done" = xyes; then
+	      AC_MSG_RESULT([yes])
+	   else
+	      AC_MSG_RESULT([no])
+	   fi
+	fi
+	
+	#
+	# Linux gcc on some architectures such as mips/mipsel forgets
+	# about -lpthread
+	#
+	if test x"$done" = xno; then
+	   AC_MSG_CHECKING([whether -lpthread fixes that])
+	   LIBS="-lpthread $PTHREAD_LIBS $save_LIBS"
+	   AC_TRY_LINK([#include <pthread.h>],
+	      [pthread_t th; pthread_join(th, 0);
+	      pthread_attr_init(0); pthread_cleanup_push(0, 0);
+	      pthread_create(0,0,0,0); pthread_cleanup_pop(0); ],
+	      [done=yes])
+	
+	   if test "x$done" = xyes; then
+	      AC_MSG_RESULT([yes])
+	      PTHREAD_LIBS="-lpthread $PTHREAD_LIBS"
+	   else
+	      AC_MSG_RESULT([no])
+	   fi
+	fi
+	#
+	# FreeBSD 4.10 gcc forgets to use -lc_r instead of -lc
+	#
+	if test x"$done" = xno; then
+	   AC_MSG_CHECKING([whether -lc_r fixes that])
+	   LIBS="-lc_r $PTHREAD_LIBS $save_LIBS"
+	   AC_TRY_LINK([#include <pthread.h>],
+	       [pthread_t th; pthread_join(th, 0);
+	        pthread_attr_init(0); pthread_cleanup_push(0, 0);
+	        pthread_create(0,0,0,0); pthread_cleanup_pop(0); ],
+	       [done=yes])
+	
+	   if test "x$done" = xyes; then
+	      AC_MSG_RESULT([yes])
+	      PTHREAD_LIBS="-lc_r $PTHREAD_LIBS"
+	   else
+	      AC_MSG_RESULT([no])
+	   fi
+	fi
+	if test x"$done" = xno; then
+	   # OK, we have run out of ideas
+	   AC_MSG_WARN([Impossible to determine how to use pthreads with shared libraries])
+	
+	   # so it's not safe to assume that we may use pthreads
+	   acx_pthread_ok=no
+	fi
+	
+	CFLAGS="$save_CFLAGS"
+	LIBS="$save_LIBS"
+	CC="$save_CC"
 else
         PTHREAD_CC="$CC"
 fi

m4/google_namespace.m4

@@ -6,6 +6,12 @@
 # when it makes sense -- for instance, when publishing stl-like code -- you
 # may want to go with a different default, like 'std'.
 
+# We guarantee the invariant that GOOGLE_NAMESPACE starts with ::,
+# unless it's the empty string.  Thus, it's always safe to do
+# GOOGLE_NAMESPACE::foo and be sure you're getting the foo that's
+# actually in the google namespace, and not some other namespace that
+# the namespace rules might kick in.
+
 AC_DEFUN([AC_DEFINE_GOOGLE_NAMESPACE],
   [google_namespace_default=[$1]
    AC_ARG_ENABLE(namespace, [  --enable-namespace=FOO to define these Google
@@ -19,7 +25,7 @@ AC_DEFUN([AC_DEFINE_GOOGLE_NAMESPACE],
                   esac],
                  [google_namespace="$google_namespace_default"])
    if test -n "$google_namespace"; then
-     ac_google_namespace="$google_namespace"
+     ac_google_namespace="::$google_namespace"
      ac_google_start_namespace="namespace $google_namespace {"
      ac_google_end_namespace="}"
    else

packages/deb/changelog

+google-gflags (0.9-1) unstable; urgency=low
+
+  * New upstream release.
+
+ -- Google Inc. <opensource@google.com>  Mon, 21 Jul 2008 23:01:38 -0700
+
 google-gflags (0.8-1) unstable; urgency=low
 
   * New upstream release.

packages/deb/libgoogle-gflags0.dirs

 usr/lib
+usr/bin

packages/deb/libgoogle-gflags0.install

 usr/lib/lib*.so.*
 debian/tmp/usr/lib/lib*.so.*
+usr/bin/*
+debian/tmp/usr/bin/*

packages/rpm/rpm.spec

@@ -56,6 +56,7 @@ rm -rf $RPM_BUILD_ROOT
 
 %{prefix}/lib/libgflags.so.0
 %{prefix}/lib/libgflags.so.0.0.0
+%{prefix}/bin/gflags_completions.sh
 
 %files devel
 %defattr(-,root,root)

python/gflags.py

@@ -190,13 +190,18 @@ try:
 except AttributeError:   # a very old python, that lacks sys.version_info
   raise NotImplementedError("requires python 2.2.0 or later")
 
-# If we're not running at least python 2.3, define True and False
+# If we're not running at least python 2.2.1, define True, False, and bool.
 # Thanks, Guido, for the code.
 try:
-  True, False
+  True, False, bool
 except NameError:
   False = 0
   True = 1
+  def bool(x):
+    if x:
+      return True
+    else:
+      return False
 
 # Are we running under pychecker?
 _RUNNING_PYCHECKER = 'pychecker.python' in sys.modules
@@ -219,7 +224,7 @@ class FlagsError(Exception):
   """The base class for all flags errors"""
 
 class DuplicateFlag(FlagsError):
-  """"Raised if there is a flag naming conflict"""
+  """Raised if there is a flag naming conflict"""
 
 # A DuplicateFlagError conveys more information than
 # a DuplicateFlag. Since there are external modules
@@ -519,7 +524,13 @@ class FlagValues:
       flag_values: registry to copy from
     """
     for flag_name, flag in flag_values.FlagDict().iteritems():
-      self[flag_name] = flag
+      # Flags with shortnames will appear here twice (once with under
+      # its normal name, and again with its short name).  To prevent
+      # problems (DuplicateFlagError) that occur when doubly
+      # registering flags, we perform a check to make sure that the
+      # entry we're looking at is for its normal name.
+      if flag_name == flag.name:
+        self[flag_name] = flag
 
   def __setitem__(self, name, flag):
     """
@@ -1103,7 +1114,6 @@ class Flag:
   def __init__(self, parser, serializer, name, default, help_string,
                short_name=None, boolean=0, allow_override=0):
     self.name = name
-    self.default = default
 
     if not help_string:
       help_string = '(no help available)'
@@ -1117,17 +1127,7 @@ class Flag:
     self.allow_override = allow_override
     self.value = None
 
-    # We can't allow a None override because it may end up not being
-    # passed to C++ code when we're overriding C++ flags.  So we
-    # cowardly bail out until someone fixes the semantics of trying to
-    # pass None to a C++ flag.  See swig_flags.Init() for details on
-    # this behavior.
-    if default is None and allow_override:
-      raise DuplicateFlag, name
-
-    self.Unparse()
-
-    self.default_as_str = self.__GetParsedValueAsString(self.value)
+    self.SetDefault(default)
 
   def __GetParsedValueAsString(self, value):
     if value is None:
@@ -1172,13 +1172,17 @@ class Flag:
     """
     Change the default value, and current value, of this flag object
     """
-    if value is not None:     # See __init__ for logic details
-      self.Parse(value)
-      self.present -= 1 # reset .present after parsing new default value
-    else:
-      self.value = None
+    # We can't allow a None override because it may end up not being
+    # passed to C++ code when we're overriding C++ flags.  So we
+    # cowardly bail out until someone fixes the semantics of trying to
+    # pass None to a C++ flag.  See swig_flags.Init() for details on
+    # this behavior.
+    if value is None and self.allow_override:
+      raise DuplicateFlag, self.name
+
     self.default = value
-    self.default_as_str = self.__GetParsedValueAsString(value)
+    self.Unparse()
+    self.default_as_str = self.__GetParsedValueAsString(self.value)
 # End of Flag definition
 
 class ArgumentParser:
@@ -1284,14 +1288,21 @@ class BooleanParser(ArgumentParser):
 
   def Convert(self, argument):
     """
-    convert the argument to a boolean (integer); raise ValueError on errors
+    convert the argument to a boolean; raise ValueError on errors
     """
     if type(argument) == str:
       if argument.lower() in ['true', 't', '1']:
-        return 1
+        return True
       elif argument.lower() in ['false', 'f', '0']:
-        return 0
-    return int(argument)
+        return False
+
+    bool_argument = bool(argument)
+    if argument == bool_argument:
+      # The argument is a valid boolean (True, False, 0, or 1), and not just
+      # something that always converts to bool (list, string, int, etc.).
+      return bool_argument
+
+    raise ValueError('Non-boolean argument to boolean flag', argument)
 
   def Parse(self, argument):
     val = self.Convert(argument)
@@ -1300,7 +1311,7 @@ class BooleanParser(ArgumentParser):
 class BooleanFlag(Flag):
   """
   A basic boolean flag.  Boolean flags do not take any arguments, and
-  their value is either 0 (false) or 1 (true).  The false value is
+  their value is either True (1) or False (0).  The false value is
   specified on the command line by prepending the word 'no' to either
   the long or short flag name.
 
@@ -1319,7 +1330,7 @@ def DEFINE_boolean(name, default, help, flag_values=FLAGS, **args):
   If a user wants to specify a false value explicitly, the long option
   beginning with 'no' must be used: i.e. --noflag
 
-  This flag will have a value of None, 0 or 1.  None is possible if
+  This flag will have a value of None, True or False.  None is possible if
   default=None and the user does not specify the flag on the command
   line.
   """

python/gflags_unittest.py

@@ -44,14 +44,50 @@ import unittest
 import gflags as flags
 FLAGS=flags.FLAGS
 
-# If we're not running at least python 2.3, as is the case when
-# invoked from flags_unittest_2_2, define True and False.
-# Thanks, Guido, for the code.
-try:
-  True, False
-except NameError:
-  False = 0
-  True = 1
+def MultiLineEqual(expected_help, help):
+  """Returns True if expected_help == help.  Otherwise returns False
+  and logs the difference in a human-readable way.
+  """
+  if help == expected_help:
+    return True
+  
+  print "Error: FLAGS.MainModuleHelp() didn't return the expected result."
+  print "Got:"
+  print help
+  print "[End of got]"
+
+  help_lines = help.split('\n')
+  expected_help_lines = expected_help.split('\n')
+
+  num_help_lines = len(help_lines)
+  num_expected_help_lines = len(expected_help_lines)
+
+  if num_help_lines != num_expected_help_lines:
+    print "Number of help lines = %d, expected %d" % (
+        num_help_lines, num_expected_help_lines)
+
+  num_to_match = min(num_help_lines, num_expected_help_lines)
+
+  for i in range(num_to_match):
+    if help_lines[i] != expected_help_lines[i]:
+      print "One discrepancy: Got:"
+      print help_lines[i]
+      print "Expected:"
+      print expected_help_lines[i]
+      break
+  else:
+    # If we got here, found no discrepancy, print first new line.
+    if num_help_lines > num_expected_help_lines:
+      print "New help line:"
+      print help_lines[num_expected_help_lines]
+    elif num_expected_help_lines > num_help_lines:
+      print "Missing expected help line:"
+      print expected_help_lines[num_help_lines]
+    else:
+      print "Bug in this test -- discrepancy detected but not found."
+
+  return False
+
 
 class FlagsUnitTest(unittest.TestCase):
   "Flags Unit Test"
@@ -551,6 +587,20 @@ class FlagsUnitTest(unittest.TestCase):
     self.assertEqual("new1" in FLAGS.FlagDict(), True)
     self.assertEqual("new2" in FLAGS.FlagDict(), True)
 
+    # Make sure AppendFlagValues works with flags with shortnames.
+    new_flags = flags.FlagValues()
+    flags.DEFINE_boolean("new3", 0, "runhelp n3", flag_values=new_flags)
+    flags.DEFINE_boolean("new4", 0, "runhelp n4", flag_values=new_flags,
+                         short_name="n4")
+    self.assertEqual(len(new_flags.FlagDict()), 3)
+    old_len = len(FLAGS.FlagDict())
+    FLAGS.AppendFlagValues(new_flags)
+    self.assertEqual(len(FLAGS.FlagDict())-old_len, 3)
+    self.assertTrue("new3" in FLAGS.FlagDict())
+    self.assertTrue("new4" in FLAGS.FlagDict())
+    self.assertTrue("n4" in FLAGS.FlagDict())
+    self.assertEqual(FLAGS.FlagDict()['n4'], FLAGS.FlagDict()['new4'])
+
     # Make sure AppendFlagValues fails on duplicates
     flags.DEFINE_boolean("dup4", 0, "runhelp d41")
     new_flags = flags.FlagValues()
@@ -586,12 +636,19 @@ class FlagsUnitTest(unittest.TestCase):
     except flags.FlagsError:
       pass
 
-    # Argument erroneously supplied for boolean
+    # Non-boolean arguments for boolean
     try:
       argv = ('./program', '--debug=goofup')
       FLAGS(argv)
-      raise AssertionError("No argument allowed exception not raised")
-    except flags.FlagsError:
+      raise AssertionError("Illegal flag value exception not raised")
+    except flags.IllegalFlagValue:
+      pass
+
+    try:
+      argv = ('./program', '--debug=42')
+      FLAGS(argv)
+      raise AssertionError("Illegal flag value exception not raised")
+    except flags.IllegalFlagValue:
       pass
 
 
@@ -667,12 +724,14 @@ class FlagsUnitTest(unittest.TestCase):
     flags.DEFINE_boolean('UnitTestBoolFlag', 0, 'Some Boolean thing')
     flags.DEFINE_integer('UnitTestNumber', 12345, 'Some integer',
                          lower_bound=0)
+    flags.DEFINE_list('UnitTestList', "1,2,3", 'Some list')
 
   def _UndeclareSomeFlags(self):
     FLAGS.__delattr__('UnitTestMessage1')
     FLAGS.__delattr__('UnitTestMessage2')
     FLAGS.__delattr__('UnitTestBoolFlag')
     FLAGS.__delattr__('UnitTestNumber')
+    FLAGS.__delattr__('UnitTestList')
 
   #### Flagfile Unit Tests ####
   def testMethod_flagfiles_1(self):
@@ -825,6 +884,13 @@ class FlagsUnitTest(unittest.TestCase):
     FLAGS([ 'dummyscript', '--UnitTestBoolFlag=true' ])
     self.assertEqual(FLAGS.UnitTestBoolFlag, True)
 
+    # Test that setting a list default works correctly.
+    FLAGS['UnitTestList'].SetDefault('4,5,6')
+    self.assertEqual(FLAGS.UnitTestList, ['4', '5', '6'])
+    self.assertEqual(FLAGS['UnitTestList'].default_as_str, "'4,5,6'")
+    FLAGS([ 'dummyscript', '--UnitTestList=7,8,9' ])
+    self.assertEqual(FLAGS.UnitTestList, ['7', '8', '9'])
+
     # Test that setting invalid defaults raises exceptions
     self.assertRaises(flags.IllegalFlagValue,
                       FLAGS['UnitTestNumber'].SetDefault, 'oops')
@@ -1120,42 +1186,7 @@ class FlagsUnitTest(unittest.TestCase):
   -z,--[no]zoom1: runhelp z1
     (default: 'false')"""
 
-    if help != expected_help:
-      print "Error: FLAGS.MainModuleHelp() didn't return the expected result."
-      print "Got:"
-      print help
-      print "[End of got]"
-
-      help_lines = help.split('\n')
-      expected_help_lines = expected_help.split('\n')
-
-      num_help_lines = len(help_lines)
-      num_expected_help_lines = len(expected_help_lines)
-
-      if num_help_lines != num_expected_help_lines:
-        print "Number of help lines = %d, expected %d" % (
-            num_help_lines, num_expected_help_lines)
-
-      num_to_match = min(num_help_lines, num_expected_help_lines)
-
-      for i in range(num_to_match):
-        if help_lines[i] != expected_help_lines[i]:
-          print "One discrepancy: Got:"
-          print help_lines[i]
-          print "Expected:"
-          print expected_help_lines[i]
-          break
-      else:
-        # If we got here, found no discrepancy, print first new line.
-        if num_help_lines > num_expected_help_lines:
-          print "New help line:"
-          print help_lines[num_expected_help_lines]
-        elif num_expected_help_lines > num_help_lines:
-          print "Missing expected help line:"
-          print expected_help_lines[num_help_lines]
-        else:
-          print "Bug in this test -- discrepancy detected but not found."
-
+    if not MultiLineEqual(expected_help, help):
       self.fail()
 
   def test_create_flag_errors(self):

src/gflags_completions.sh

+#!/bin/bash
+
+# Copyright (c) 2008, Google Inc.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+# ---
+# Author: Dave Nicponski
+#
+# This script is invoked by bash in response to a matching compspec.  When
+# this happens, bash calls this script using the command shown in the -C
+# block of the complete entry, but also appends 3 arguments.  They are:
+#   - The command being used for completion
+#   - The word being completed
+#   - The word preceding the completion word.
+#
+# Here's an example of how you might use this script:
+# $ complete -o bashdefault -o default -o nospace -C                         \
+#   '/usr/local/bin/gflags_completions.sh --tab_completion_columns $COLUMNS' \
+#   time  env  binary_name  another_binary  [...]
+
+# completion_word_index gets the index of the (N-1)th argument for
+# this command line.  completion_word gets the actual argument from
+# this command line at the (N-1)th position
+completion_word_index="$(($# - 1))"
+completion_word="${!completion_word_index}"
+
+# TODO(daven): Replace this once commandlineflags_completions.cc has
+# a bool parameter indicating unambiguously to hijack the process for
+# completion purposes.
+if [ -z "$completion_word" ]; then
+  # Until an empty value for the completion word stops being misunderstood
+  # by google3 binaries, don't actuall execute the binary or the process
+  # won't be hijacked!
+  exit 0
+fi
+
+# binary_index gets the index of the command being completed (which bash
+# places in the (N-2)nd position.  binary gets the actual command from
+# this command line at that (N-2)nd position
+binary_index="$(($# - 2))"
+binary="${!binary_index}"
+
+# For completions to be universal, we may have setup the compspec to
+# trigger on 'harmless pass-through' commands, like 'time' or 'env'.
+# If the command being completed is one of those two, we'll need to
+# identify the actual command being executed.  To do this, we need
+# the actual command line that the <TAB> was pressed on.  Bash helpfully
+# places this in the $COMP_LINE variable.
+if [ "$binary" == "time" ] || [ "$binary" == "env" ]; then
+  # we'll assume that the first 'argument' is actually the
+  # binary to be run, if we think it looks like a google3
+  # binary
+
+  # TODO(daven): Decide what 'looks' like a google3 binary.  =)
+
+  # TODO(daven): This is not perfect - the 'env' command, for instance,
+  #   is allowed to have options between the 'env' and 'the command to
+  #   be executed'.  For example, consider:
+  # $ env FOO="bar"  bin/do_something  --help<TAB>
+  # In this case, we'll mistake the FOO="bar" portion as the binary.
+  #   Perhaps we should continuing consuming leading words until we
+  #   either run out of words, or find a word that is a valid file
+  #   marked as executable.  I can't think of any reason this wouldn't
+  #   work.
+
+  # Break up the 'original command line' (not this script's command line,
+  # rather the one the <TAB> was pressed on) and find the second word.
+  parts=( ${COMP_LINE} )
+  binary=${parts[1]}
+fi
+
+# Build the command line to use for completion.  Basically it involves
+# passing through all the arguments given to this script (except the 3
+# that bash added), and appending a '--tab_completion_word "WORD"' to
+# the arguments.
+params=""
+for ((i=1; i<=$(($# - 3)); ++i)); do 
+  params="$params \"${!i}\"";
+done
+params="$params --tab_completion_word \"$completion_word\""
+
+# TODO(daven): Perhaps stash the output in a temporary file somewhere
+# in /tmp, and only cat it to stdout if the command returned a success
+# code, to prevent false positives
+
+# If we think we have a reasonable command to execute, then execute it
+# and hope for the best.
+if [ -f "$binary" ] && [ -x "$binary" ]; then
+  eval "$binary 2>/dev/null $params"
+fi
+

src/gflags_reporting.cc

@@ -56,6 +56,7 @@
 #include <string>
 #include <vector>
 #include "google/gflags.h"
+#include "google/gflags_completions.h"
 
 #ifndef PATH_SEPARATOR
 #define PATH_SEPARATOR  '/'
@@ -345,6 +346,8 @@ void HandleCommandLineHelpFlags() {
   const char* progname = ProgramInvocationShortName();
   extern void (*commandlineflags_exitfunc)(int);   // in gflags.cc
 
+  HandleCommandLineCompletions();
+
   if (FLAGS_helpshort) {
     // show only flags related to this binary:
     // E.g. for fileutil.cc, want flags containing   ... "/fileutil." cc

src/gflags_unittest.sh

@@ -150,8 +150,8 @@ Expect $LINENO 1 "/gflags_unittest.cc" "/gflags.cc" \
   --helpon gflags_unittest
 
 # helpmatch is like helpon but takes substrings
-Expect $LINENO 1 "/gflags_unittest.cc" "/gflags.cc" \
-  -helpmatch _
+Expect $LINENO 1 "/gflags_reporting.cc" "/gflags_unittest.cc" \
+  -helpmatch reporting
 Expect $LINENO 1 "/gflags_unittest.cc" "/gflags.cc" \
   -helpmatch=unittest
 
@@ -208,12 +208,15 @@ Expect $LINENO 0 "gflags_unittest" "gflags_unittest.cc" \
 Expect $LINENO 0 "PASS" "" -- --help
 
 # Make sure boolean flags gives warning when type of default value is not bool
-Expect $LINENO 0 "Flag test_bool_string is of type bool, but its default value is not a boolean."
-Expect $LINENO 0 "Flag test_bool_float is of type bool, but its default value is not a boolean."
-Expect $LINENO 0 "Flag test_bool_int is of type bool, but its default value is not a boolean."
+Expect $LINENO 0 "Flag test_bool_string is of type bool, but its default value is not a boolean." ""
+Expect $LINENO 0 "Flag test_bool_float is of type bool, but its default value is not a boolean." ""
+Expect $LINENO 0 "Flag test_bool_int is of type bool, but its default value is not a boolean." ""
 
 # Make sure that boolean flags don't give warning when default value is bool
 Expect $LINENO 0 "" "Flag test_bool_bool is of type bool, but its default value is not a boolean."
 
+# And we should die if the flag value doesn't pas the validator
+Expect $LINENO 1 "ERROR: failed validation of new value 'true' for flag 'always_fail'" "" --always_fail
+
 echo "PASS"
 exit 0

src/google/gflags_completions.h.in

+// Copyright (c) 2008, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+// ---
+// Author: Dave Nicponski
+//
+// Implement helpful bash-style command line flag completions
+//
+// ** Functional API:
+// HandleCommandLineCompletions() should be called early during
+// program startup, but after command line flag code has been
+// initialized, such as the beginning of HandleCommandLineHelpFlags().
+// It checks the value of the flag --tab_completion_word.  If this
+// flag is empty, nothing happens here.  If it contains a string,
+// however, then HandleCommandLineCompletions() will hijack the
+// process, attempting to identify the intention behind this
+// completion.  Regardless of the outcome of this deduction, the
+// process will be terminated, similar to --helpshort flag
+// handling.
+//
+// ** Overview of Bash completions:
+// Bash can be told to programatically determine completions for the
+// current 'cursor word'.  It does this by (in this case) invoking a
+// command with some additional arguments identifying the command
+// being executed, the word being completed, and the previous word
+// (if any).  Bash then expects a sequence of output lines to be
+// printed to stdout.  If these lines all contain a common prefix
+// longer than the cursor word, bash will replace the cursor word
+// with that common prefix, and display nothing.  If there isn't such
+// a common prefix, bash will display the lines in pages using 'more'.
+//
+// ** Strategy taken for command line completions:
+// If we can deduce either the exact flag intended, or a common flag
+// prefix, we'll output exactly that.  Otherwise, if information
+// must be displayed to the user, we'll take the opportunity to add
+// some helpful information beyond just the flag name (specifically,
+// we'll include the default flag value and as much of the flag's
+// description as can fit on a single terminal line width, as specified
+// by the flag --tab_completion_columns).  Furthermore, we'll try to
+// make bash order the output such that the most useful or relevent
+// flags are the most likely to be shown at the top.
+//
+// ** Additional features:
+// To assist in finding that one really useful flag, substring matching
+// was implemented.  Before pressing a <TAB> to get completion for the
+// current word, you can append one or more '?' to the flag to do
+// substring matching.  Here's the semantics:
+//   --foo<TAB>     Show me all flags with names prefixed by 'foo'
+//   --foo?<TAB>    Show me all flags with 'foo' somewhere in the name
+//   --foo??<TAB>   Same as prior case, but also search in module
+//                  definition path for 'foo'
+//   --foo???<TAB>  Same as prior case, but also search in flag
+//                  descriptions for 'foo'
+// Finally, we'll trim the output to a relatively small number of
+// flags to keep bash quiet about the verbosity of output.  If one
+// really wanted to see all possible matches, appending a '+' to the
+// search word will force the exhaustive list of matches to be printed.
+//
+// ** How to have bash accept completions from a binary:
+// Bash requires that it be informed about each command that programmatic
+// completion should be enabled for.  Example addition to a .bashrc
+// file would be (your path to gflags_completions.sh file may differ):
+
+/*
+$ complete -o bashdefault -o default -o nospace -C                        \
+ '/usr/local/bin/gflags_completions.sh --tab_completion_columns $COLUMNS' \
+  time  env  binary_name  another_binary  [...]
+*/
+
+// This would allow the following to work:
+//   $ /path/to/binary_name --vmodule<TAB>
+// Or:
+//   $ ./bin/path/another_binary --gfs_u<TAB>
+// (etc)
+//
+// Sadly, it appears that bash gives no easy way to force this behavior for
+// all commands.  That's where the "time" in the above example comes in.
+// If you haven't specifically added a command to the list of completion
+// supported commands, you can still get completions by prefixing the
+// entire command with "env".
+//   $ env /some/brand/new/binary --vmod<TAB>
+// Assuming that "binary" is a newly compiled binary, this should still
+// produce the expected completion output.
+
+
+#ifndef GOOGLE_GFLAGS_COMPLETIONS_H_
+#define GOOGLE_GFLAGS_COMPLETIONS_H_
+
+@ac_google_start_namespace@
+
+void HandleCommandLineCompletions(void);
+
+@ac_google_end_namespace@
+
+#endif  // GOOGLE_GFLAGS_COMPLETIONS_H_