This is the mail archive of the insight@sources.redhat.com mailing list for the Insight project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: I wish....


 
> W.r.t the checking in of your patch, we need to consult the other maintainers.
> 
> We haven't established a police regarding if we check in the new code
> commented out as you suggested of if we just keep it with the PR.
> In gdb core land it would be the latter, but I particularly think that
> in our case, at the least in the present moment, it would be better to
> have it checked in.  It would get documented in CVS and would help people
> who are working in improving the code to see it in place.
> 
> Jim and Syd, which approach do you prefer, the former or the latter?

In general, I don't like commented out code. Even if the code works for
something and left commented out for a reason, it is a strong candidate
for bit rot.

However, if the developer will never see the code because it is buried
in a PR, then it is better to leave a detailed comment in the code with
an explanation as to what has to happen for the code to make it in.
 
> > 2000-12-02  Tom Tromey  <tromey@redhat.com>
> >
> >         * console.ith (_set_wrap): Declare.
> >         (_update_option): Likewise.
> >         * console.itb (Console::constructor): Install preference hooks.
> >         (Console::destructor): Remove preference hooks.
> >         (Console::_set_wrap): New method.
> >         (Console::_update_option): New method.
> >         (Console::_build_win): Use _set_wrap.
> >
> > Tom
> >
> > Index: console.itb
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbtk/library/console.itb,v
> > retrieving revision 1.4
> > diff -u -r1.4 console.itb
> > --- console.itb 2000/11/01 22:15:37     1.4
> > +++ console.itb 2000/12/02 23:25:19
> > @@ -22,6 +22,12 @@
> >    add_hook gdb_busy_hook [list $this busy]
> >    add_hook gdb_idle_hook [list $this idle]
> >    add_hook gdb_no_inferior_hook [list $this idle]
> > +
> > +  foreach option {gdb/console/wrap gdb/console/prompt_fg \
> > +                   gdb/console/error_fg gdb/console/font} {
> > +    pref add_hook $option [code $this _update_option]
> > +  }
> > +
> >    set gdbtk_state(console) $this
> >  }
> >
> > @@ -31,26 +37,20 @@
> >    remove_hook gdb_busy_hook [list $this busy]
> >    remove_hook gdb_idle_hook [list $this idle]
> >    remove_hook gdb_no_inferior_hook [list $this idle]
> > -}
> > -
> > -body Console::_build_win {} {
> > -  set wrap [pref get gdb/console/wrap]
> >
> > -  if { $wrap } {
> > -    set hsm none
> > -  } else {
> > -    set hsm dynamic
> > +  foreach option {gdb/console/wrap gdb/console/prompt_fg \
> > +                   gdb/console/error_fg gdb/console/font} {
> > +    pref remove_hook $option [code $this _update_option]
> >    }
> > -  iwidgets::scrolledtext $itk_interior.stext -hscrollmode $hsm \
> > +}
> > +
> > +body Console::_build_win {} {
> > +  iwidgets::scrolledtext $itk_interior.stext \
> >      -vscrollmode dynamic -textbackground white
> >
> >    set _twin [$itk_interior.stext component text]
> >
> > -  if {$wrap} {
> > -    $_twin configure -wrap word
> > -  } else {
> > -    $_twin configure -wrap none
> > -  }
> > +  _set_wrap [pref get gdb/console/wrap]
> >
> >    $_twin tag configure prompt_tag -foreground [pref get gdb/console/prompt_fg]
> >    $_twin tag configure err_tag -foreground [pref get gdb/console/error_fg]
> > @@ -613,4 +613,44 @@
> >  body Console::_reset_tab {} {
> >    bind $_twin <KeyPress> {}
> >    set _saw_tab 0
> > +}
> > +
> > +
> > +# ------------------------------------------------------------------
> > +#  METHOD:  _set_wrap - Set wrap mode
> > +# ------------------------------------------------------------------
> > +body Console::_set_wrap {wrap} {
> > +  if { $wrap } {
> > +    set hsm none
> > +    set wv char
> > +  } else {
> > +    set hsm dynamic
> > +    set wv none
> > +  }
> > +
> > +  $itk_interior.stext configure -hscrollmode $hsm
> > +  $_twin configure -wrap $wv
> > +}
> > +
> > +# ------------------------------------------------------------------
> > +#  METHOD:  _update_option - Update in response to preference change
> > +# ------------------------------------------------------------------
> > +body Console::_update_option {name value} {
> > +  switch -- $name {
> > +    gdb/console/wrap {
> > +      _set_wrap $value
> > +    }
> > +
> > +    gdb/console/prompt_fg {
> > +      $_twin tag configure prompt_tag -foreground $value
> > +    }
> > +
> > +    gdb/console/error_fg {
> > +      $_twin tag configure err_tag -foreground $value
> > +    }
> > +
> > +    gdb/console/font {
> > +      $_twin configure -font $value
> > +    }
> > +  }
> >  }
> > Index: console.ith
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/gdbtk/library/console.ith,v
> > retrieving revision 1.1.1.1
> > diff -u -r1.1.1.1 console.ith
> > --- console.ith 2000/02/07 00:19:42     1.1.1.1
> > +++ console.ith 2000/12/02 23:25:19
> > @@ -61,5 +61,7 @@
> >      method _search_history {}
> >      method _rsearch_history {}
> >      method _setprompt {{prompt {}}}
> > +    method _set_wrap {wrap}
> > +    method _update_option {name value}
> >    }
> >  }
> 
> --
> Fernando Nasser
> Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario   M4P 2C9

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]