changeset 32052:d77b987a7e0d

Provide more informative error messages for various bad calling forms to set() (bug #64072) * graphics.cc (graphics_object::set (const octave_value_list& args)): Clarify summary of function. Remove unnecessary FIXME. Move BIST tests to below Fset function. * graphics.cc (Fset): Check if first argument is a string and process as NAME/VALUE pair. Provide explicit error message about what is wrong if only NAME is given and no matching VALUE. Directly call set_value_or_default() rather than forming a 2-element octave_value_list and calling set() function which accepts octave_value_list objects. Provide explicit error message if cell array of PROPERTIES is given with no following cell array of VALUES. If input validation fails just report "invalid syntax" rather than trying to suggest which input was at fault.
author Rik <rik@octave.org>
date Tue, 25 Apr 2023 07:50:29 -0700
parents 17a09d2bbe0f
children 5998f6639148
files libinterp/corefcn/graphics.cc
diffstat 1 files changed, 54 insertions(+), 51 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/graphics.cc	Sun Apr 23 11:08:16 2023 +0200
+++ b/libinterp/corefcn/graphics.cc	Tue Apr 25 07:50:29 2023 -0700
@@ -2550,7 +2550,7 @@
   return m;
 }
 
-// Set properties given as a cs-list of name, value pairs.
+// Set property given as either cs-list of name/value pairs or a struct.
 
 void
 graphics_object::set (const octave_value_list& args)
@@ -2562,9 +2562,6 @@
 
   for (int i = 0; i < nargin; )
     {
-      // FIXME: Should this if branch be eliminated and determination of
-      // struct input be determined earlier such that the correct set(...)
-      // function is invoked by the compiler?
       if (args(i).isstruct ())
         {
           set (args(i).map_value ());
@@ -2679,41 +2676,6 @@
 }
 
 /*
-## test set ticklabels for compatibility
-%!test
-%! hf = figure ("visible", "off");
-%! set (gca (), "xticklabel", [0, 0.2, 0.4, 0.6, 0.8, 1]);
-%! xticklabel = get (gca (), "xticklabel");
-%! close (hf);
-%! assert (class (xticklabel), "char");
-%! assert (size (xticklabel), [6, 3]);
-
-%!test
-%! hf = figure ("visible", "off");
-%! set (gca (), "xticklabel", "0|0.2|0.4|0.6|0.8|1");
-%! xticklabel = get (gca (), "xticklabel");
-%! close (hf);
-%! assert (class (xticklabel), "char");
-%! assert (size (xticklabel), [6, 3]);
-
-%!test
-%! hf = figure ("visible", "off");
-%! set (gca (), "xticklabel", ["0 "; "0.2"; "0.4"; "0.6"; "0.8"; "1 "]);
-%! xticklabel = get (gca (), "xticklabel");
-%! close (hf);
-%! assert (class (xticklabel), "char");
-%! assert (size (xticklabel), [6, 3]);
-
-%!test
-%! hf = figure ("visible", "off");
-%! set (gca (), "xticklabel", {"0", "0.2", "0.4", "0.6", "0.8", "1"});
-%! xticklabel = get (gca (), "xticklabel");
-%! close (hf);
-%! assert (class (xticklabel), "cell");
-%! assert (size (xticklabel), [6, 1]);
-*/
-
-/*
 ## test set with struct arguments
 %!test
 %! hf = figure ("visible", "off");
@@ -12261,8 +12223,19 @@
       // Loop over input arguments
       for (octave_idx_type i = 1; i < args.length (); )
         {
-          if ((i < nargin - 1) && args(i).iscellstr () && args(i+1).iscell ())
-            {
+          if (args(i).is_string ())
+            {
+              if (i == nargin - 1)
+                error ("set: PROPERTY name must be followed by a VALUE");
+              const caseless_str pname = args(i).string_value ();
+              const octave_value val = args(i+1);
+              go.set_value_or_default (pname, val);
+              i += 2;
+            }
+          else if (args(i).iscellstr ())
+            {
+              if ((i == nargin - 1) || ! args(i+1).iscell ())
+                error ("set: cell array of PROPERTIES must be followed by cell array of VALUES");
               if (args(i+1).cell_value ().rows () == 1)
                 go.set (args(i).cellstr_value (), args(i+1).cell_value (), 0);
               else if (hcv.numel () == args(i+1).cell_value ().rows ())
@@ -12275,17 +12248,12 @@
               i += 2;
             }
           else if (args(i).isstruct ())
-          {
-            go.set (args(i).map_value ());
-            i += 1;
-          }
-          else if (i < nargin - 1)
-          {
-            go.set (args.slice (i, 2));
-            i += 2;
-          }
+            {
+              go.set (args(i).map_value ());
+              i += 1;
+            }
           else
-            error ("set: invalid syntax at input #%" OCTAVE_IDX_TYPE_FORMAT, i+1);
+            error ("set: invalid syntax");
         }
 
       request_drawnow = true;
@@ -12297,6 +12265,41 @@
   return retval;
 }
 
+/*
+## test setting ticklabels for compatibility
+%!test
+%! hf = figure ("visible", "off");
+%! set (gca (), "xticklabel", [0, 0.2, 0.4, 0.6, 0.8, 1]);
+%! xticklabel = get (gca (), "xticklabel");
+%! close (hf);
+%! assert (class (xticklabel), "char");
+%! assert (size (xticklabel), [6, 3]);
+
+%!test
+%! hf = figure ("visible", "off");
+%! set (gca (), "xticklabel", "0|0.2|0.4|0.6|0.8|1");
+%! xticklabel = get (gca (), "xticklabel");
+%! close (hf);
+%! assert (class (xticklabel), "char");
+%! assert (size (xticklabel), [6, 3]);
+
+%!test
+%! hf = figure ("visible", "off");
+%! set (gca (), "xticklabel", ["0 "; "0.2"; "0.4"; "0.6"; "0.8"; "1 "]);
+%! xticklabel = get (gca (), "xticklabel");
+%! close (hf);
+%! assert (class (xticklabel), "char");
+%! assert (size (xticklabel), [6, 3]);
+
+%!test
+%! hf = figure ("visible", "off");
+%! set (gca (), "xticklabel", {"0", "0.2", "0.4", "0.6", "0.8", "1"});
+%! xticklabel = get (gca (), "xticklabel");
+%! close (hf);
+%! assert (class (xticklabel), "cell");
+%! assert (size (xticklabel), [6, 1]);
+*/
+
 static std::string
 get_graphics_object_type (double val)
 {