# HG changeset patch
# User John W. Eaton <jwe@octave.org>
# Date 1251402189 14400
# Node ID bdcfb756d721d173e387cebc1724237f656e98da
# Parent  3d0d2bda3a0fccfe99d04b64e7488daa6d81a56f
improve error messages for ambiguous graphics property names

diff -r 3d0d2bda3a0f -r bdcfb756d721 liboctave/ChangeLog
--- a/liboctave/ChangeLog	Thu Aug 27 16:08:23 2009 +0200
+++ b/liboctave/ChangeLog	Thu Aug 27 15:43:09 2009 -0400
@@ -1,3 +1,8 @@
+2009-08-27  John W. Eaton  <jwe@octave.org>
+
+	* str-vec.cc, str-vec.h (string_vector::string_vector (const
+	std::set<std::string>&)): New constructor.
+
 2009-08-27  Jaroslav Hajek  <highegg@gmail.com>
 
 	* mx-inlines.cc (DEFCMPLXCMOP): Remove.
diff -r 3d0d2bda3a0f -r bdcfb756d721 liboctave/str-vec.cc
--- a/liboctave/str-vec.cc	Thu Aug 27 16:08:23 2009 +0200
+++ b/liboctave/str-vec.cc	Thu Aug 27 15:43:09 2009 -0400
@@ -40,6 +40,9 @@
 #include "lo-utils.h"
 #include "str-vec.h"
 
+// FIXME -- isn't there some STL trick that could be used to make this
+// work for all STL containers of std::string objects?
+
 string_vector::string_vector (const std::list<std::string>& lst)
   : Array<std::string> ()
 {
@@ -55,6 +58,21 @@
     elem(i++) = *p;
 }
 
+string_vector::string_vector (const std::set<std::string>& lst)
+  : Array<std::string> ()
+{
+  size_t n = lst.size ();
+
+  resize (n);
+
+  octave_idx_type i = 0;
+
+  for (std::set<std::string>::const_iterator p = lst.begin ();
+       p != lst.end ();
+       p++)
+    elem(i++) = *p;
+}
+
 // Create a string vector from a NULL terminated list of C strings.
 
 string_vector::string_vector (const char * const *s)
diff -r 3d0d2bda3a0f -r bdcfb756d721 liboctave/str-vec.h
--- a/liboctave/str-vec.h	Thu Aug 27 16:08:23 2009 +0200
+++ b/liboctave/str-vec.h	Thu Aug 27 15:43:09 2009 -0400
@@ -26,6 +26,7 @@
 
 #include <iosfwd>
 #include <list>
+#include <set>
 #include <string>
 
 #include "Array.h"
@@ -48,6 +49,8 @@
 
   string_vector (const std::list<std::string>& lst);
 
+  string_vector (const std::set<std::string>& lst);
+
   string_vector (const char * const *s);
 
   string_vector (const char * const *s, octave_idx_type n);
diff -r 3d0d2bda3a0f -r bdcfb756d721 src/ChangeLog
--- a/src/ChangeLog	Thu Aug 27 16:08:23 2009 +0200
+++ b/src/ChangeLog	Thu Aug 27 15:43:09 2009 -0400
@@ -1,3 +1,13 @@
+2009-08-27  John W. Eaton  <jwe@octave.org>
+
+	* graphics.cc (validate_property_name): New static function.
+	(base_properties::dynamic_property_names): New function.
+	* graphicffs.h.in (base_properties::dynamic_property_names):
+	Provide decl.
+	* genprops.awk: Generate all_property_names functions.  Use
+	all_property_names as needed to avoid duplication elsewhere.
+	Call validate_property_name in set/get functions.
+
 2009-08-27  Jaroslav Hajek  <highegg@gmail.com>
 
 	* ov-class.cc (get_current_method_class): Simplify.
diff -r 3d0d2bda3a0f -r bdcfb756d721 src/genprops.awk
--- a/src/genprops.awk	Thu Aug 27 16:08:23 2009 +0200
+++ b/src/genprops.awk	Thu Aug 27 15:43:09 2009 -0400
@@ -259,9 +259,9 @@
 
   printf ("public:\n\n");
   if (base)
-    printf ("\n  static bool has_property (const std::string& pname, const std::string& cname);\n\n");
+    printf ("\n  static std::set<std::string> all_property_names (const std::string& cname);\n\n  static bool has_property (const std::string& pname, const std::string& cname);\n\n");
   else
-    printf ("\n  static bool has_property (const std::string& pname);\n\n");
+    printf ("\n  static std::set<std::string> all_property_names (void);\n\n  static bool has_property (const std::string& pname);\n\n");
 
   if (idx > 0)
     print (base ? "protected:\n" : "private:\n");
@@ -433,6 +433,9 @@
       printf ("void\n%s::properties::set (const caseless_str& pname, const octave_value& val)\n{\n",
               class_name) >> filename;
 
+    if (! base)
+      printf ("  const std::set<std::string>& pnames = all_property_names ();\n\n  validate_property_name (\"get\", pnames, pname);\n\n  if (error_state)\n    return;\n\n") >> filename;
+
     first = 1;
 
     for (i = 1; i <= idx; i++)
@@ -474,7 +477,7 @@
     }
 
     printf ("\n  return m;\n}\n\n") >> filename;
-    
+
     ## get "one" method
 
     if (base)
@@ -484,6 +487,9 @@
               class_name) >> filename;
     printf ("  octave_value retval;\n\n") >> filename;
 
+    if (! base)
+      printf ("  const std::set<std::string>& pnames = all_property_names ();\n\n  validate_property_name (\"get\", pnames, pname);\n\n  if (error_state)\n    return retval;\n\n") >> filename;
+
     for (i = 1; i<= idx; i++)
     {
       printf ("  %sif (pname.compare (\"%s\"))\n",
@@ -506,6 +512,9 @@
       printf ("property\n%s::properties::get_property (const caseless_str& pname)\n{\n",
               class_name) >> filename;
 
+    if (! base)
+      printf ("  const std::set<std::string>& pnames = all_property_names ();\n\n  validate_property_name (\"get\", pnames, pname);\n\n  if (error_state)\n    return property ();\n\n") >> filename;
+
     for (i = 1; i<= idx; i++)
     {
       if (ptype[i])
@@ -567,17 +576,24 @@
               class_name, object_name) >> filename;
 
     if (base)
-      printf ("bool base_properties::has_property (const std::string& pname, const std::string& cname") >> filename;
+      printf ("std::set<std::string>\nbase_properties::all_property_names (const std::string& cname") >> filename;
     else
-    printf ("bool %s::properties::has_property (const std::string& pname", class_name) >> filename;
-    printf (")\n{\n  static std::set<std::string> all_properties;\n\n  static bool initialized = false;\n\n  if (! initialized)\n    {\n") >> filename;
+      printf ("std::set<std::string>\n%s::properties::all_property_names (void", class_name) >> filename;
+    printf (")\n{\n  static std::set<std::string> all_pnames;\n\n  static bool initialized = false;\n\n  if (! initialized)\n    {\n") >> filename;
     for (i = 1; i <= idx; i++)
-      printf ("      all_properties.insert (\"%s\");\n", name[i]) >> filename;
+      printf ("      all_pnames.insert (\"%s\");\n", name[i]) >> filename;
     printf ("\n      initialized = true;\n    }\n\n") >> filename;
     if (base)
-	printf ("  return all_properties.find (pname) != all_properties.end () || has_dynamic_property (pname, cname);\n}\n\n") >> filename;
+      printf ("  std::set<std::string> retval = all_pnames;\n  std::set<std::string> dyn_props = dynamic_property_names (cname);\n  retval.insert (dyn_props.begin(), dyn_props.end ());\n  return retval;\n}\n\n") >> filename;
     else
-      printf ("  return all_properties.find (pname) != all_properties.end () || base_properties::has_property (pname, \"%s\");\n}\n\n", class_name) >> filename;
+      printf ("  std::set<std::string> retval = all_pnames;\n  std::set<std::string> base_props = base_properties::all_property_names (\"%s\");\n  retval.insert (base_props.begin (), base_props.end ());\n  return retval;\n}\n\n", class_name) >> filename;
+
+    if (base)
+      printf ("bool\nbase_properties::has_property (const std::string& pname, const std::string& cname)\n{\n  std::set<std::string> pnames = all_property_names (cname);\n\n") >> filename;
+    else
+      printf ("bool\n%s::properties::has_property (const std::string& pname)\n{\n  std::set<std::string> pnames = all_property_names ();\n\n", class_name) >> filename;
+
+    printf ("  return pnames.find (pname) != pnames.end ();\n}\n\n") >> filename;
   }
 }
 
diff -r 3d0d2bda3a0f -r bdcfb756d721 src/graphics.cc
--- a/src/graphics.cc	Thu Aug 27 16:08:23 2009 +0200
+++ b/src/graphics.cc	Thu Aug 27 15:43:09 2009 -0400
@@ -61,6 +61,49 @@
   error ("set: invalid value for %s property", pname.c_str ());
 }
 
+static void
+validate_property_name (const std::string& who,
+			const std::set<std::string>& pnames,
+			const caseless_str& pname)
+{
+  size_t len = pname.length ();
+  std::set<std::string> matches;
+
+  for (std::set<std::string>::const_iterator p = pnames.begin ();
+       p != pnames.end (); p++)
+    {
+      if (pname.compare (*p, len))
+	matches.insert (*p);
+    }
+
+  size_t num_matches = matches.size ();
+
+  if (num_matches == 0)
+    {
+      error ("%s: unknown property %s", who.c_str (), pname.c_str ());
+    }
+  else if (num_matches > 1)
+    {
+      string_vector sv (matches);
+
+      std::ostringstream os;
+
+      sv.list_in_columns (os);
+
+      std::string match_list = os.str ();
+
+      error ("%s: ambiguous property name %s; possible matches:\n\n%s",
+	     who.c_str (), pname.c_str (), match_list.c_str ());
+    }
+  else if (num_matches == 1 && ! pname.compare (*(matches.begin ())))
+    {
+      std::string possible_match = *(matches.begin ());
+
+      error ("%s: instead of %s, did you mean %s?",
+	     who.c_str (), pname.c_str (), possible_match.c_str ());
+    }
+}
+
 static Matrix
 jet_colormap (void)
 {
@@ -1742,17 +1785,17 @@
 
 std::map<std::string, std::set<std::string> > base_properties::all_dynamic_properties;
 
+std::set<std::string>
+base_properties::dynamic_property_names (const std::string& cname)
+{
+  return all_dynamic_properties[cname];
+}
+
 bool
 base_properties::has_dynamic_property (const std::string& pname,
 				       const std::string& cname)
 {
-  // FIXME -- we need to maintain a static map of class names to sets
-  // of dynamic property names, then look up the set for the given
-  // cname, then see if the set contains the given pname.  Doing that
-  // implies changes to set_dynamic, I think.  Where is set_dynamic
-  // ever used?
-
-  std::set<std::string>& dynprops = all_dynamic_properties[cname];
+  const std::set<std::string>& dynprops = dynamic_property_names (cname);
 
   return dynprops.find (pname) != dynprops.end ();
 }
@@ -5537,10 +5580,7 @@
   octave_value retval;
 
   if (obj)
-    {
-      caseless_str p = std::string (property);
-      retval = obj.get (p);
-    }
+    retval = obj.get (caseless_str (property));
   else
     error ("%s: invalid handle (= %g)", func.c_str(), handle);
 
@@ -5558,9 +5598,9 @@
 
   if (obj)
     {
-      caseless_str p = std::string (property);
-      obj.set (p, arg);
-      if (!error_state)
+      obj.set (caseless_str (property), arg);
+
+      if (! error_state)
 	ret = true;
     }
   else
diff -r 3d0d2bda3a0f -r bdcfb756d721 src/graphics.h.in
--- a/src/graphics.h.in	Thu Aug 27 16:08:23 2009 +0200
+++ b/src/graphics.h.in	Thu Aug 27 15:43:09 2009 -0400
@@ -1732,6 +1732,8 @@
 
   static std::map<std::string, std::set<std::string> > all_dynamic_properties;
  
+  static std::set<std::string> dynamic_property_names (const std::string& cname);
+
   static bool has_dynamic_property (const std::string& pname,
 				    const std::string& cname);