[Chicago-talk] Help hacking some code.

[Shlomi Fish]

Hi!

See below for my response.

On Friday 02 Oct 2009 14:50:53 Richard Reina wrote:
> For a number of years I have been using Steve Kunz perlmenu.pm as a nice
>  low overhead GUI for a database app.  I am very happy with it.  However, I
>  am try to dig deep into its code to solve a mystery. I have written the
>  author and have not heard back so I was hoping that perhaps someone here
>  might lend some advice. The code below provides a basic menu and I am
>  trying to add color by placing
> 
> use Term::ANSIColor;
> print color 'bold blue';

I don't think Term::ANSIColor works well with Curses.pm. You should try:

http://www.perlmonks.org/?node_id=32154

(first hit for http://www.google.com/search?q=curses%20perl%20color )

Other than that, I should note that the module you are using seems to be coded 
quite amateurishly. Let me go over that: 

> 
> in different parts of the code in order to try an add color. No matter
>  where I seem to place it, no color is displayed. I have even tried placing
>  it in the menu_display subroutine -- which is called in the code -- in
>  perlmenu.pm.  Please find that subroutine further down.  All of the code
>  in perlmenu.pm is posted here
>  http://kobesearch.cpan.org/htdocs/perlmenu/perlmenu.pm.html

non-Pragma modules should start with an uppercase letter - PerlMenu.pm or 
Perlmenu.pm or whatever. And it should be under a more meaningful namespace 
than the top-level one.

> 
> For several days I have been searching -- to no avail -- for some kind of
>  system("reset") in the code that would blow away color.  Is this something
>  that Curses is canceling out?  From what I understand perl Curses does
>  support color, so I am confused about what is preventing this from
>  working.  Does anyone have any ideas as to how I can go about achieving
>  this task?
> 
> Any help would be greatly appreciated and if there are any MLB fans
>  listening I will gladly donate my 4 seats down the third base line for
>  today's 1:20 game against the Arizona Diamondbacks in exchange for help
>  solving this problem.
> 
> BEGIN { $Curses::OldCurses = 1; }
> use Curses;
> use perlmenu;
> ...
> &menu_init(1,"Select an Animal"); # Init menu

You shouldn't use &menu_init(...) with an ampersand - use menu_init(...) 
instead.

> 
> &menu_item("Collie","dog"); # Add item
> &menu_item("Shetland","pony"); # Add item
> &menu_item("Persian","cat"); # Add last item
> 
> $sel = &menu_display("Which animal?"); # Get user selection
> 
> if ($sel eq "%UP%") { ... }
> if ($sel eq "dog") { ... }

Maybe use a dispatch table here.

> 
> 
> 
> #**********
> #  MENU_DISPLAY
> #
> #  Function:	Display items in menu_sel_text array, allow selection, and
> #		return appropriate selection-string.
> #
> #  Call format:	$sel = &menu_display("Prompt text",$arrow_line,$top_item);
> #
> #  Arguments:   - Prompt text (for the bottom line of menu).
> #		- Line number offset for the arrow (defaults to zero).
> #		- Index of top item on screen (defaults to zero).
> #		- Column number offset for the arrow (multiple-column mode
> #		  only, defaults to zero).
> #
> #  Returns:     Selected action string (from second param on &menu_item)
> #		%UP%    -- "u"|"U" pressed (or "t"|"T" and looking for top)
> #               %EMPTY% -- Nothing in menu to display
> #
> #  Notes:	1) This routine ALWAYS sets "nocbreak" and "echo" terminal
> #		   modes before returning.
> #		2) This routine exits directly (after calling the optional
> #		   "quit" routine) if "q"|"Q" is pressed.
> #**********
> sub menu_display {
>   ($menu_prompt,$arrow_spec_row,$menu_top_item,$arrow_spec_col) = @_;

You should use my for these variables them. And use strict and use warnings

>   local($ret);

Why are you using local?

> 
> # Check for no "menu_item" calls.
>   $total_items = $#menu_sel_text + 1;

$total_items should be declared.

>   if ($total_items <= 0) {
>     &nocbreak();		# ALWAYS turn off "cbreak" mode
>     &echo();		# ALWAYS turn on "echo"
>     return("%EMPTY%");
>   }
> 
>   &clear(); $xrow = $xcol = 0;

Don't put them both on the same line.

>   $last_menu_top_item = -1;	# Force drawing of menu items
>   $ret = &menu_display_internal(0,$menu_prompt,0);
>   if ($#_ > 0) { $_[1] = $arrow_spec_row; }

Why are you accessing @_ directly using $#_ like that?

>   if ($#_ > 1) { $_[2] = $menu_top_item; }
>   if ($#_ > 2) { $_[3] = $arrow_spec_col; }
>   &menu_return_prep();
>   $ret;

You should separate logical paragraphs of your code using empty lines.

And << $ret; >> should be << return $ret; >>.

For more information, consult:

http://oreilly.com/catalog/9780596001735

Regards,

	Shlomi Fish


-- 
-----------------------------------------------------------------
Shlomi Fish       http://www.shlomifish.org/
Best Introductory Programming Language - http://shlom.in/intro-lang

Chuck Norris read the entire English Wikipedia in 24 hours. Twice.