rkumar / rbcurse

ruby based curses widgets: fields, buttons, textarea. menus, message boxes, tabbed panes, tables, listboxes, splitpanes, scrollpanes. Event based, MVC architecture. (DEPRECATED - PLS USE rbcurse-core)
http://totalrecall.wordpress.com/
138 stars 15 forks source link

@toprow attribute not properly cleared #15

Closed mande closed 11 years ago

mande commented 11 years ago

Hi,

After calling the list method of an instance of the List class and change data list, @toprow attribute is not cleared, so code crashed when calling mvwchgat.

To check this:

Thanks

rkumar commented 11 years ago

Corrected. If the previously highlighted row has gone out of focus, then should not attempt to highlight since the visibleindex is nil. (I don't think it has anything to do with @toprow, can you tell me how you got that impression ?

I have committed the fix.

rkumar commented 11 years ago

Maybe you meant @current_index which should have been set to 0. When i checked the code of rlist.list it does set the current_index to zero.

mande commented 11 years ago

Hi,

This is exception dump ( copy & paste):

/usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/system/window.rb:215:in mvwchgat': no implicit conversion from nil to integer (TypeError) from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/system/window.rb:215:inmethod_missing' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/include/listscrollable.rb:661:in highlight_focussed_row' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rlist.rb:381:inon_enter_row' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rlist.rb:374:in on_enter' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rwidget.rb:1511:inon_enter' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rwidget.rb:1530:in select_field' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rwidget.rb:1599:inblock in select_next_field' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rwidget.rb:1595:in upto' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rwidget.rb:1595:inselect_next_field' from /usr/local/lib/ruby/gems/1.9.1/gems/rbcurse-core-0.0.13/lib/rbcurse/core/widgets/rwidget.rb:1873:in handle_key' from rbcurse3.rb:146:inblock in

' from rbcurse3.rb:139:in catch' from rbcurse3.rb:139:in
'

Exception trigger is mvwchgat which is receiving nil in his first parameter, that is ' row' value, and this come from 'highlight_focussed_row' function. Row value is calculated through _convert_index_to_printable_row -> _convert_index_to_visible_row as: pos = index - @toprow return nil if pos < 0 || pos > scrollatrow()

as index=0 and @toprow > 0, result is negative and nil is returned.

As a workaround, call '.init_vars' method after changing list data does the work.

Thanks

rkumar commented 11 years ago

Good catch, i had followed the stack-trace back to the same methods but missed toprow there somehow. Yes, list() should call init_vars() but I am just playing safe by setting toprow and pcol to zero in list(). Thanks a lot.

mande commented 11 years ago

I meant that I used the call to init_vars as a workaround to avoid the error and continue testing the gem without changing code.

I'm trying to use it for a project with handsets ( data capture, wifi + ssh + curses, no heavy battery-eaters graphics), for my company, not commercial software. I had to play around with the gem, but I think I've got enough knowledge to work with.

Thanks for your work

rkumar commented 11 years ago

Yes, I had understood that you used init_vars. Let me know if i you have any issues.

I am currently using a much simpler table widget that is based on textpad which is itself a much simpler version of textarea. I will upload the table widget to rbcurse-experimental -- if you are interested you can use it -- then i will move it to core afer using it myself for a while.

rkumar commented 11 years ago

i am also now using the much simpler textpad as a list, so that obviates having to use two separate widgets (textview and list). I was testing this out in a gem named cygnus and it worked well.