Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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.
  • Loading branch information...
commit 4953c44b539a53767c2742d4b17e5c477471d88c 1 parent 4419d3d
David Benjamin authored February 22, 2013
24  perl/lib/BarnOwl.pm
@@ -436,9 +436,8 @@ sub new_variable_int {
436 436
             %{$args},
437 437
             get_tostring => sub { "$storage" },
438 438
             set_fromstring => sub {
439  
-                return -1 unless $_[0] =~ /^-?[0-9]+$/;
  439
+                die "Expected integer" unless $_[0] =~ /^-?[0-9]+$/;
440 440
                 $storage = 0 + $_[0];
441  
-                return 0;
442 441
             },
443 442
             validsettings => "<int>",
444 443
             takes_on_off => 0,
@@ -452,9 +451,8 @@ sub new_variable_bool {
452 451
             %{$args},
453 452
             get_tostring => sub { $storage ? "on" : "off" },
454 453
             set_fromstring => sub {
455  
-                return -1 unless $_[0] eq "on" || $_[0] eq "off";
  454
+                die "Valid settings are on/off" unless $_[0] eq "on" || $_[0] eq "off";
456 455
                 $storage = $_[0] eq "on";
457  
-                return 0;
458 456
             },
459 457
             validsettings => "on,off",
460 458
             takes_on_off => 1,
@@ -467,10 +465,7 @@ sub new_variable_string {
467 465
     BarnOwl::new_variable_full($name, {
468 466
             %{$args},
469 467
             get_tostring => sub { $storage },
470  
-            set_fromstring => sub {
471  
-                $storage = $_[0];
472  
-                return 0;
473  
-            },
  468
+            set_fromstring => sub { $storage = $_[0]; },
474 469
             validsettings => "<string>",
475 470
             takes_on_off => 0,
476 471
         });
@@ -491,9 +486,8 @@ sub new_variable_enum {
491 486
             %{$args},
492 487
             get_tostring => sub { $storage },
493 488
             set_fromstring => sub {
494  
-                return -1 unless $valid{$_[0]};
  489
+                die "Invalid input" unless $valid{$_[0]};
495 490
                 $storage = $_[0];
496  
-                return 0;
497 491
             },
498 492
             validsettings => join(",", @{$args->{validsettings}})
499 493
         });
@@ -513,7 +507,7 @@ Create a variable, in full generality. The keyword arguments have types below:
513 507
 
514 508
 The get/set functions are required. Note that the caller manages storage for the
515 509
 variable. get_tostring/set_fromstring both convert AND store the value.
516  
-set_fromstring returns 0 on success.
  510
+set_fromstring dies on failure.
517 511
 
518 512
 If the variable takes parameters 'on' and 'off' (i.e. is boolean-looking), set
519 513
 takes_on_off to 1. This makes :set VAR and :unset VAR work. set_fromstring will
@@ -538,7 +532,13 @@ sub new_variable_full {
538 532
     my $get_tostring_fn = sub { $args{get_tostring}->() };
539 533
     my $set_fromstring_fn = sub {
540 534
       my ($dummy, $val) = @_;
541  
-      $args{set_fromstring}->($val);
  535
+      # Translate from user-supplied die-on-failure callback to expected
  536
+      # non-zero on error. Less of a nuisance than interacting with ERRSV.
  537
+      eval { $args{set_fromstring}->($val) };
  538
+      # TODO: Consider changing B::I::new_variable to expect string|NULL with
  539
+      # string as the error message. That can then be translated to a GError in
  540
+      # owl_variable_set_fromstring. For now the string is ignored.
  541
+      return ($@ ? -1 : 0);
542 542
     };
543 543
 
544 544
     BarnOwl::Internal::new_variable($name, $args{summary}, $args{description}, $args{validsettings},
3  t/variable.t
@@ -61,9 +61,8 @@ BarnOwl::new_variable_full("fullvar", {
61 61
     validsettings => '<short-words>',
62 62
     get_tostring => sub { "value is " . $value },
63 63
     set_fromstring => sub {
64  
-	return -1 unless $_[0] =~ /^...?$/;
  64
+	die "Too long" unless $_[0] =~ /^...?$/;
65 65
 	$value = $_[0];
66  
-	return 0;
67 66
     },
68 67
     takes_on_off => 1
69 68
 });

0 notes on commit 4953c44

Please sign in to comment.
Something went wrong with that request. Please try again.