On Wednesday 04 April 2012 16:13:43 Dan Fandrich wrote:
> On Wed, Apr 04, 2012 at 02:38:42PM +0200, Kamil Dudka wrote:
> > I was more thinking of a new option for this. Nevertheless, detecting
> > term capabilities might also be a solution. The problem is that we need
> > to link an additional library to access this information. Or am I
> > missing something?
>
> That's where tgetstr() and tigetstr() are on my Linux box, too.

Thanks for the review. I think we first need to reach an agreement that
detecting term capabilities is really the preferred way to solve the problem.

> > Please have a look at the attached patch. Any feedback is welcome.
> >
> > +AC_ARG_WITH(ncurses,
> > +AC_HELP_STRING([--with-ncurses],[Enable ncurses support if available]),
> > + [OPT_NCURSES=$withval])
> > +
> > +if test "x$OPT_NCURSES" != xno; then
> > + if test "x$OPT_NCURSES" = xyes; then
> > + CURL_CHECK_PKGCONFIG(ncurses)
> > + if test "$PKGCONFIG" != "no" ; then
> > + addlib=`$PKGCONFIG --libs ncurses`
> > + addcflags=`$PKGCONFIG --cflags ncurses`
>
> Does curses even come with a pkg-config file?

At least on Fedora and RHEL-6, it does.

> > +#ifdef USE_NCURSES
> > +# include <term.h>
> > +#endif
>
> My docs say that both term.h and curses.h need to be included.
>
> > + if(bar->dummy_term)
> > + fputs(line, bar->out);
> > + else {
> > + snprintf(format, sizeof(format), "\r%%-%ds %%5.1f%%%%", barwidth);
>
> Since we've gone through the effort to read the 'cr' capability out of
> terminfo, we might as well use it here in place of a hard-coded \r.

The idea behind was not to touch something that works. Are there any known
issues with the current solution? If not, I would just keep it working. It
can be easily extended later on.

> > +#ifdef USE_NCURSES
> > + term = curlx_getenv("TERM");
> > + if(term) {
> > + bar->dummy_term = (1 != tgetent(NULL, term)) || !tgetstr("cr",
> > NULL);
>
> What is the purpose of using the termcap compatibility routines here
> instead of the terminfo routines? termcap has been obsolete for a long,
> long time.

No purpose. I am just not familiar with those APIs.

> > + free(term);
> > + }
> > +#endif
> > +
> > bar->out = config->errors;
> > }
> >
> > diff --git a/src/tool_cb_prg.h b/src/tool_cb_prg.h
> > index f64335a..c0e29fd 100644
> > --- a/src/tool_cb_prg.h
> > +++ b/src/tool_cb_prg.h
> > @@ -32,6 +32,8 @@ struct ProgressData {
> > int width;
> > FILE *out; /* where to write everything to */
> > curl_off_t initial_size;
> > + bool dummy_term;
>
> dumb_term or dumber_term here sounds better to my ears.

Yep, dumb_term sounds better.

Kamil
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-users
FAQ: http://curl.haxx.se/docs/faq.html
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2012-04-04