Changeset 4953c44

Timestamp:

Feb 23, 2013, 5:44:23 PM (12 years ago)

Author:

David Benjamin <davidben@mit.edu>

Branches:

master, release-1.10

Children:

6a8b519

Parents:

4419d3d

git-author:

David Benjamin <davidben@mit.edu> (02/22/13 14:12:23)

git-committer:

David Benjamin <davidben@mit.edu> (02/23/13 17:44:23)

Message:

Change set_fromstring to die on failure

Returning zero/non-zero is awkward. Leave a note for future improvement
to pass the error message along, although the default one is pretty
reasonable already.

Files:

• perl/lib/BarnOwl.pm (modified) (6 diffs)
• t/variable.t (modified) (1 diff)

perl/lib/BarnOwl.pm

@@ -437 +437 @@
             get_tostring => sub { "$storage" },
             set_fromstring => sub {
-                return -1 unless $_[0] =~ /^-?[0-9]+$/;
+                die "Expected integer" unless $_[0] =~ /^-?[0-9]+$/;
                 $storage = 0 + $_[0];
-                return 0;
             },
             validsettings => "<int>",
@@ -453 +452 @@
             get_tostring => sub { $storage ? "on" : "off" },
             set_fromstring => sub {
-                return -1 unless $_[0] eq "on" || $_[0] eq "off";
+                die "Valid settings are on/off" unless $_[0] eq "on" || $_[0] eq "off";
                 $storage = $_[0] eq "on";
-                return 0;
             },
             validsettings => "on,off",
@@ -468 +466 @@
             %{$args},
             get_tostring => sub { $storage },
-            set_fromstring => sub {
-                $storage = $_[0];
-                return 0;
-            },
+            set_fromstring => sub { $storage = $_[0]; },
             validsettings => "<string>",
             takes_on_off => 0,
@@ -492 +487 @@
             get_tostring => sub { $storage },
             set_fromstring => sub {
-                return -1 unless $valid{$_[0]};
+                die "Invalid input" unless $valid{$_[0]};
                 $storage = $_[0];
-                return 0;
             },
             validsettings => join(",", @{$args->{validsettings}})
@@ -514 +508 @@
 The get/set functions are required. Note that the caller manages storage for the
 variable. get_tostring/set_fromstring both convert AND store the value.
-set_fromstring returns 0 on success.
+set_fromstring dies on failure.
 
 If the variable takes parameters 'on' and 'off' (i.e. is boolean-looking), set
@@ -539 +533 @@
     my $set_fromstring_fn = sub {
       my ($dummy, $val) = @_;
-      $args{set_fromstring}->($val);
+      # Translate from user-supplied die-on-failure callback to expected
+      # non-zero on error. Less of a nuisance than interacting with ERRSV.
+      eval { $args{set_fromstring}->($val) };
+      # TODO: Consider changing B::I::new_variable to expect string|NULL with
+      # string as the error message. That can then be translated to a GError in
+      # owl_variable_set_fromstring. For now the string is ignored.
+      return ($@ ? -1 : 0);
     };
 

t/variable.t

@@ -62 +62 @@
     get_tostring => sub { "value is " . $value },
     set_fromstring => sub {
-        return -1 unless $_[0] =~ /^...?$/;
+        die "Too long" unless $_[0] =~ /^...?$/;
         $value = $_[0];
-        return 0;
     },
     takes_on_off => 1