eserte / perl-tk

the perl module Tk
https://metacpan.org/release/Tk
Other
44 stars 31 forks source link

menu demos: use empty string to clear $status_bar #34

Closed chrstphrchvz closed 3 years ago

chrstphrchvz commented 6 years ago

Under Tcl::pTk, using undef instead of an empty string to clear $status_bar (the textvariable for the status bar label) can cause a "Use of uninitialized value in subroutine entry" warning.

I haven't seen this warning under Perl/Tk, but am providing this for consistency. I don't know whether a label should support textvariable being set to undef and act the same as when set to empty string.

eserte commented 4 years ago

Actually I think that clearing $status_bar here and setting it in the next step (without any Tk event handling in between) is a no-op. Probably this line can be removed completely. Do you agree?

diff --git i/demos/demos/widget_lib/menus.pl w/demos/demos/widget_lib/menus.pl
index 10fa7bf..e13e952 100644
--- i/demos/demos/widget_lib/menus.pl
+++ w/demos/demos/widget_lib/menus.pl
@@ -173,7 +173,6 @@ sub menus {
         -font => 'Helvetica 10', -textvariable => \$status_bar)->
        pack(qw/-padx 2 -pady 2 -expand yes -fill both/);
     $menubar->bind('<<MenuSelect>>' => sub {
-       $status_bar = undef;
        $status_bar = $_[0]->entrycget('active', -label);
        $TOP->idletasks;
     });
chrstphrchvz commented 4 years ago

Actually I think that clearing $status_bar here and setting it in the next step (without any Tk event handling in between) is a no-op. Probably this line can be removed completely. Do you agree?

I do like the idea of removing the line better. It turns out that since opening this PR I had done so in cleaning up the corresponding lines in Tcl::pTk: https://github.com/chrstphrchvz/perl-tcl-ptk/commit/0da2a41b5ab158cf63cd535ae63929d3d7276b38 I have revised this PR to be consistent with those changes.

The important part I intended to keep for consistency (but which my original attempt of this PR was missing) is using $textvariable = ... || '' to prevent uninitialized value warnings (which I now think might be caused by use warnings present in some of Tcl::pTk, but not Perl/Tk). Unless, again, someone believes it is a mistake for Tcl::pTk to emit this warning and require $textvariable = ... || '' in cases where Perl/Tk doesn't complain.

Edit: see chrstphrchvz/perl-tcl-ptk#17. I believe the cause is in Tcl.xs.