pcp
[Top] [All Lists]

Re: [pcp] readline support for dbpmda

To: kenj@xxxxxxxxxxxxxxxx, Andrew Wansink <ajwans@xxxxxxxxxxx>
Subject: Re: [pcp] readline support for dbpmda
From: Nathan Scott <nscott@xxxxxxxxxx>
Date: Wed, 20 Jan 2010 12:39:26 +1100 (EST)
Cc: pcp@xxxxxxxxxxx
In-reply-to: <1263932134.18485.22.camel@xxxxxxxxxxxxxxxx>
----- "Ken McDonell" <kenj@xxxxxxxxxxxxxxxx> wrote:

> ...
> I'll commit this one to my tree this evening if I don't see any
> adverse
> comments in the interim.
> 
> Thanks again to Andrew for getting the heavy lifting done here.

Some small review comments...                                             

--- a/configure.in
+++ b/configure.in
 ...              
+savedLIBS=$LIBS  
+LIBS=            
+dnl Need flex if we are to get a lexer which can read from a buffer
+if test $LEX = flex                                                
+then                                                               
+    lib_for_curses=                                                
+    lib_for_readline=                                              
...                                                                 
+    AC_SUBST(lib_for_readline)                                     
+    AC_SUBST(lib_for_curses)                                       
+fi                                                                 
+LIBS=$savedLIBS                                                    

This bit looks not-quite-right - the LIBS=/savedLIBS= part
should be inside "if test $LEX" and the lib_for_curses= , 
lib_for_readline= and AC_SUBST lines should be outside the
conditional (so that the SUBST is always done - otherwise 
we end up with @lib_for_curses@ in the generated builddefs
for platforms with no support.                            

Here...

--- a/src/dbpmda/src/dbpmda.c
+++ b/src/dbpmda/src/dbpmda.c
@@ -78,7 +78,14 @@ main(int argc, char **argv)
 #endif                                       

        case 'e':               /* echo input */
+#if HAVE_READLINE
 ...

And here...

--- a/src/dbpmda/src/lex.l
+++ b/src/dbpmda/src/lex.l
@@ -24,6 +24,13 @@ int lineno = 0;

 #include "./lex.h"

+#if HAVE_READLINE
+#include <readline/readline.h>
+#include <readline/history.h>

(and one or two more places) ... we use "#if HAVE_READLINE",
should be "#ifdef" I suspect - for platforms with no support
it will be undef (not zero).

+#ifndef HAVE_READLINE
+                   if ( iflag ) {
+                       printf("%s> ", prompt);
+                       fflush(stdout);
                    }
+#endif

... the above is fine & should stay as is - its more along
the lines of what the code usually does with configure macros.


Thanks for this, BTW - I'll certainly be using it!

cheers.

-- 
Nathan

<Prev in Thread] Current Thread [Next in Thread>