more richio enhancements, /new work, SCH::LIB_TABLE nearly finished

CHANGELOG.txt

@@ -4,6 +4,20 @@ KiCad ChangeLog 2010
 Please add newer entries at the top, list the date and your name with
 email address.
 
+2010-Dec-28 UPDATE Dick Hollenbeck <dick@softplc.com>
+================================================================================
+++new:
+    Completed a good portion of /new class LIB_TABLE.
+    Starting LPID.
+++common:
+    Tricked xnode.h into not issuing deprecation warnings.
+++richio:
+  * Added support of DSNLEXER( LINE_READER* ) to TokenList2DsnLexer.cmake, which
+    allows the chaining of different grammars on top of a common LINE_READER.
+  * Changed OUTPUT_FORMATTER::Quoted() to return a std::string and not modify
+    its input parameter.
+
+
 2010-dec-21 UPDATE Wayne Stambaugh <stambaughw@verizon.net>
 ================================================================================
 ++all

CMakeModules/TokenList2DsnLexer.cmake

@@ -223,8 +223,9 @@ using namespace DSN;    // enum ${enum} is in this namespace
 class ${RESULT}_LEXER : public DSNLEXER
 {
 public:
+
     /**
-     * Constructor ${RESULT}_LEXER
+     * Constructor ( const std::string&, const wxString& )
      * @param aSExpression is (utf8) text possibly from the clipboard that you want to parse.
      * @param aSource is a description of the origin of @a aSExpression, such as a filename.
      *   If left empty, then _("clipboard") is used.
@@ -236,7 +237,7 @@ public:
     }
 
     /**
-     * Constructor ${RESULT}_LEXER
+     * Constructor ( FILE* )
      * takes @a aFile already opened for reading and @a aFilename as parameters.
      * The opened file is assumed to be positioned at the beginning of the file
      * for purposes of accurate line number reporting in error messages.  The
@@ -245,11 +246,28 @@ public:
      * @param aFilename is the name of the opened file, needed for error reporting.
      */
     ${RESULT}_LEXER( FILE* aFile, const wxString& aFilename ) :
-        DSNLEXER( DSN::${result}_keywords,  DSN::${result}_keyword_count,
+        DSNLEXER( DSN::${result}_keywords, DSN::${result}_keyword_count,
                   aFile, aFilename )
     {
     }
 
+    /**
+     * Constructor ( LINE_READER* )
+     * intializes a lexer and prepares to read from @a aLineReader which
+     * is assumed ready, and may be in use by other DSNLEXERs also.  No ownership
+     * is taken of @a aLineReader. This enables it to be used by other lexers also.
+     * The transition between grammars in such a case, must happen on a text
+     * line boundary, not within the same line of text.
+     *
+     * @param aLineReader is any subclassed instance of LINE_READER, such as
+     *  STRING_LINE_READER or FILE_LINE_READER.  No ownership is taken of aLineReader.
+     */
+    ${RESULT}_LEXER( LINE_READER* aLineReader ) :
+        DSNLEXER( DSN::${result}_keywords, DSN::${result}_keyword_count,
+                  aLineReader )
+    {
+    }
+
     /**
      * Function NextTok
      * returns the next token found in the input file or T_EOF when reaching

common/dsnlexer.cpp

@@ -62,6 +62,7 @@ void DSNLEXER::init()
 
 DSNLEXER::DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
                     FILE* aFile, const wxString& aFilename ) :
+    iOwnReaders( true ),
     keywords( aKeywordTable ),
     keywordCount( aKeywordCount )
 {
@@ -73,6 +74,7 @@ DSNLEXER::DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
 
 DSNLEXER::DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
                     const std::string& aClipboardTxt, const wxString& aSource ) :
+    iOwnReaders( true ),
     keywords( aKeywordTable ),
     keywordCount( aKeywordCount )
 {
@@ -83,6 +85,28 @@ DSNLEXER::DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
 }
 
 
+DSNLEXER::DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
+                    LINE_READER* aLineReader ) :
+    iOwnReaders( false ),
+    keywords( aKeywordTable ),
+    keywordCount( aKeywordCount )
+{
+    PushReader( aLineReader );
+    init();
+}
+
+
+DSNLEXER::~DSNLEXER()
+{
+    if( iOwnReaders )
+    {
+        // delete the LINE_READERs from the stack, since I own them.
+        for( READER_STACK::iterator it = readerStack.begin(); it!=readerStack.end();  ++it )
+            delete *it;
+    }
+}
+
+
 void DSNLEXER::PushReader( LINE_READER* aLineReader )
 {
     readerStack.push_back( aLineReader );
@@ -102,7 +126,7 @@ bool DSNLEXER::PopReader()
     {
         readerStack.pop_back();
 
-        reader = &readerStack.back();
+        reader = readerStack.back();
         start  = (char*) (*reader);
 
         // force a new readLine() as first thing.

common/richio.cpp

@@ -278,30 +278,32 @@ int OUTPUTFORMATTER::Print( int nestLevel, const char* fmt, ... ) throw( IO_ERRO
 }
 
 
-const char* OUTPUTFORMATTER::Quoted( std::string* aWrapee ) throw( IO_ERROR )
+std::string OUTPUTFORMATTER::Quoted( const std::string& aWrapee ) throw( IO_ERROR )
 {
     // derived class's notion of what a quote character is
     char quote          = *GetQuoteChar( "(" );
 
     // Will the string be wrapped based on its interior content?
-    const char* squote  = GetQuoteChar( aWrapee->c_str() );
+    const char* squote  = GetQuoteChar( aWrapee.c_str() );
+
+    std::string wrapee  = aWrapee;  // return this
 
     // Search the interior of the string for 'quote' chars
     // and replace them as found with duplicated quotes.
     // Note that necessarily any string which has internal quotes will
     // also be wrapped in quotes later in this function.
-    for( unsigned i=0;  i<aWrapee->size();  ++i )
+    for( unsigned i=0;  i<wrapee.size();  ++i )
     {
-        if( (*aWrapee)[i] == quote )
+        if( wrapee[i] == quote )
         {
-            aWrapee->insert( aWrapee->begin()+i, quote );
+            wrapee.insert( wrapee.begin()+i, quote );
             ++i;
         }
-        else if( (*aWrapee)[0]=='\r' || (*aWrapee)[0]=='\n' )
+        else if( wrapee[i]=='\r' || wrapee[i]=='\n' )
         {
             // In a desire to maintain accurate line number reporting within DSNLEXER
             // a decision was made to make all S-expression strings be on a single
-            // line.  You can embedd \n (human readable) in the text but not
+            // line.  You can embed \n (human readable) in the text but not
             // '\n' which is 0x0a.
             throw IO_ERROR( _( "S-expression string has newline" ) );
         }
@@ -310,11 +312,11 @@ const char* OUTPUTFORMATTER::Quoted( std::string* aWrapee ) throw( IO_ERROR )
     if( *squote )
     {
         // wrap the beginning and end of the string in a quote.
-        aWrapee->insert( aWrapee->begin(), quote );
-        aWrapee->insert( aWrapee->end(), quote );
+        wrapee.insert( wrapee.begin(), quote );
+        wrapee.insert( wrapee.end(), quote );
     }
 
-    return aWrapee->c_str();
+    return wrapee;
 }
 
 

common/xnode.cpp

@@ -50,17 +50,14 @@ void XNODE::Format( OUTPUTFORMATTER* out, int nestLevel ) throw( IO_ERROR )
 
 void XNODE::FormatContents( OUTPUTFORMATTER* out, int nestLevel ) throw( IO_ERROR )
 {
-    std::string utf8;
-
     // output attributes first if they exist
     for( XATTR* attr = (XATTR*) GetAttributes();  attr;  attr = (XATTR*) attr->GetNext() )
     {
-        utf8  = CONV_TO_UTF8( attr->GetValue() );   // capture the content
-
         out->Print( 0, " (%s %s)",
             // attr names should never need quoting, no spaces, we designed the file.
             CONV_TO_UTF8( attr->GetName() ),
-            out->Quoted( &utf8 ) );
+            out->Quoted( CONV_TO_UTF8( attr->GetValue() ) ).c_str()
+            );
     }
 
     // we only expect to have used one of two types here:
@@ -85,8 +82,9 @@ void XNODE::FormatContents( OUTPUTFORMATTER* out, int nestLevel ) throw( IO_ERRO
         break;
 
     case wxXML_TEXT_NODE:
-        utf8  = CONV_TO_UTF8( GetContent() );
-        out->Print( 0, " %s", out->Quoted( &utf8 ) );
+        out->Print( 0, " %s",
+            out->Quoted( CONV_TO_UTF8( GetContent() ) ).c_str()
+            );
         break;
 
     default:

include/dsnlexer.h

@@ -27,9 +27,7 @@
 
 #include <cstdio>
 #include <string>
-#include <boost/ptr_container/ptr_vector.hpp>
-
-//#include "fctsys.h"
+#include <vector>
 
 #include "richio.h"
 
@@ -78,14 +76,15 @@ enum DSN_SYNTAX_T {
  */
 class DSNLEXER
 {
+    bool                iOwnReaders;            ///< on readerStack, should I delete them?
     char*               start;
     char*               next;
     char*               limit;
 
-    typedef boost::ptr_vector<LINE_READER>  READER_STACK;
+    typedef std::vector<LINE_READER*>  READER_STACK;
 
-    READER_STACK        readerStack;            ///< owns all the LINE_READERs by pointer.
-    LINE_READER*        reader;                 ///< no ownership. ownership is via readerStack.
+    READER_STACK        readerStack;            ///< all the LINE_READERs by pointer.
+    LINE_READER*        reader;                 ///< no ownership. ownership is via readerStack, maybe, if iOwnReaders
     int                 stringDelimiter;
     bool                space_in_quoted_tokens; ///< blank spaces within quoted strings
     bool                commentsAreTokens;      ///< true if should return comments as tokens
@@ -155,7 +154,7 @@ class DSNLEXER
 public:
 
     /**
-     * Constructor DSNLEXER
+     * Constructor ( FILE*, const wxString& )
      * intializes a DSN lexer and prepares to read from aFile which
      * is already open and has aFilename.
      *
@@ -168,12 +167,35 @@ public:
     DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
               FILE* aFile, const wxString& aFileName );
 
+    /**
+     * Constructor ( std::string&*, const wxString& )
+     * intializes a DSN lexer and prepares to read from @a aSExpression.
+     *
+     * @param aKeywordTable is an array of KEYWORDS holding \a aKeywordCount.  This
+     *  token table need not contain the lexer separators such as '(' ')', etc.
+     * @param aKeywordTable is the count of tokens in aKeywordTable.
+     * @param aSExpression is text to feed through a STRING_LINE_READER
+     * @param aSource is a description of aSExpression, used for error reporting.
+     */
     DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
-              const std::string& aClipboardTxt, const wxString& aSource = wxEmptyString );
+              const std::string& aSExpression, const wxString& aSource = wxEmptyString );
 
-    ~DSNLEXER()
-    {
-    }
+    /**
+     * Constructor ( LINE_READER* )
+     * intializes a DSN lexer and prepares to read from @a aLineReader which
+     * is already open, and may be in use by other DSNLEXERs also.  No ownership
+     * is taken of @a aLineReader. This enables it to be used by other DSNLEXERs also.
+     *
+     * @param aKeywordTable is an array of KEYWORDS holding \a aKeywordCount.  This
+     *  token table need not contain the lexer separators such as '(' ')', etc.
+     * @param aKeywordTable is the count of tokens in aKeywordTable.
+     * @param aLineReader is any subclassed instance of LINE_READER, such as
+     *  STRING_LINE_READER or FILE_LINE_READER.
+     */
+    DSNLEXER( const KEYWORD* aKeywordTable, unsigned aKeywordCount,
+              LINE_READER* aLineReader );
+
+    virtual ~DSNLEXER();
 
     /**
      * Function PushReader

include/kicad_exceptions.h

@@ -42,6 +42,15 @@ struct IO_ERROR
 {
     wxString    errorText;
 
+    /**
+     * Constructor ( const wxChar* )
+     * handles the case where _() is passed as aMsg.
+     */
+    IO_ERROR( const wxChar* aMsg ) :
+        errorText( aMsg )
+    {
+    }
+
     IO_ERROR( const wxString& aMsg ) :
         errorText( aMsg )
     {

include/richio.h

@@ -336,15 +336,14 @@ public:
      *
      * @param aWrapee is a string that might need wraping in double quotes,
      *  and it might need to have its internal quotes doubled up, or not.
-     *  Caller's copy may be modified, or not.
      *
-     * @return const char* - useful for passing to printf() style functions that
-     *  must output utf8 streams.
+     * @return std::string - whose c_str() function can be called for passing
+     *   to printf() style functions that must output utf8 encoded s-expression streams.
      * @throw IO_ERROR, if aWrapee has any \r or \n bytes in it which is
      *        illegal according to the DSNLEXER who does not ever want them
      *        within a string.
      */
-    virtual const char* Quoted( std::string* aWrapee )  throw( IO_ERROR );
+     virtual std::string Quoted( const std::string& aWrapee ) throw( IO_ERROR );
 
     //-----</interface functions>-----------------------------------------
 };

include/xnode.h

@@ -26,6 +26,12 @@
  */
 
 #include "richio.h"
+
+// quiet the deprecated warnings with 3 lines:
+#include <wx/defs.h>
+#undef wxDEPRECATED
+#define wxDEPRECATED(x) x
+
 #include <wx/xml/xml.h>
 
 

new/sch_dir_lib_source.cpp

@@ -50,7 +50,6 @@ using namespace SCH;
 #include <sys/stat.h>
 #include <cstring>
 #include <cstdio>
-#include <ctype.h>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -66,6 +65,7 @@ using namespace std;
 /// implementation, and to a corresponding LIB_SINK.
 /// Core EESCHEMA should never have to see this.
 #define SWEET_EXT       ".part"
+#define SWEET_EXTZ      (sizeof(SWEET_EXT)-1)
 
 
 /* __func__ is C99 prescribed, but just in case:
@@ -141,6 +141,12 @@ static const char* strrstr( const char* haystack, const char* needle )
 }
 
 
+static inline bool isDigit( char c )
+{
+    return c >= '0' && c <= '9';
+}
+
+
 /**
  * Function endsWithRev
  * returns a pointer to the final string segment: "revN[N..]" or NULL if none.
@@ -154,7 +160,7 @@ static const char* endsWithRev( const char* start, const char* tail, char separa
 {
     bool    sawDigit = false;
 
-    while( tail > start && isdigit( *--tail ) )
+    while( tail > start && isDigit( *--tail ) )
     {
         sawDigit = true;
     }
@@ -237,7 +243,7 @@ bool DIR_LIB_SOURCE::makePartName( STRING* aPartName, const char* aEntry,
         // If versioning, then must find a trailing "revN.." type of string.
         if( useVersioning )
         {
-            const char* rev = endsWithRev( cp + sizeof(SWEET_EXT) - 1, limit, '.' );
+            const char* rev = endsWithRev( cp + SWEET_EXTZ, limit, '.' );
             if( rev )
             {
                 if( aCategory.size() )
@@ -397,7 +403,7 @@ void DIR_LIB_SOURCE::GetCategoricalPartNames( STRINGS* aResults, const STRING& a
 
         while( it != limit )
         {
-            const char* rev   = endsWithRev( *it, '/' );
+            const char* rev = endsWithRev( *it, '/' );
 
             // all cached partnames have a rev string in useVersioning mode
             assert( rev );

new/sch_lib_table.cpp

@@ -25,6 +25,7 @@
 
 #include <sch_lib_table.h>
 #include <sch_lib_table_lexer.h>
+#include <set>
 
 using namespace std;
 using namespace SCH;
@@ -156,26 +157,63 @@ void LIB_TABLE::Format( OUTPUTFORMATTER* out, int nestLevel ) const
     out->Print( nestLevel, ")\n" );
 }
 
+
 void LIB_TABLE::ROW::Format( OUTPUTFORMATTER* out, int nestLevel ) const
     throw( IO_ERROR )
 {
-    out->Print( nestLevel, "(lib (logical \"%s\")(type \"%s\")(full_uri \"%s\")(options \"%s\"))\n",
-        logicalName.c_str(), libType.c_str(), fullURI.c_str(), options.c_str() );
+    out->Print( nestLevel, "(lib (logical %s)(type %s)(full_uri %s)(options %s))\n",
+        out->Quoted( logicalName ).c_str(),
+        out->Quoted( libType ).c_str(),
+        out->Quoted( fullURI ).c_str(),
+        out->Quoted( options ).c_str()
+        );
+}
+
+
+STRINGS LIB_TABLE::GetLogicalLibs()
+{
+    // only return unique logical library names.  Use std::set::insert() to
+    // quietly reject any duplicates, which can happen in the fall back table(s).
+    set<STRING>     unique;
+    STRINGS         ret;
+
+    const LIB_TABLE* cur = this;
+
+    do
+    {
+        for( ROWS_CITER it = cur->rows.begin();  it!=cur->rows.end();  ++it )
+        {
+            unique.insert( it->second->logicalName );
+        }
+
+    } while( ( cur = cur->fallBack ) != 0 );
+
+    // return a sorted, unique set of STRINGS to caller
+    for( set<STRING>::const_iterator it = unique.begin();  it!=unique.end();  ++it )
+        ret.push_back( *it );
+
+    return ret;
 }
 
 
-const LIB_TABLE::ROW* LIB_TABLE::FindRow( const STRING& aLogicalName )
+PART* LIB_TABLE::GetPart( const LPID& aLogicalPartID ) throw( IO_ERROR )
+{
+    // need LIPD done.
+    return 0;
+}
+
+
+const LIB_TABLE::ROW* LIB_TABLE::FindRow( const STRING& aLogicalName ) const
 {
     // this function must be *super* fast, so therefore should not instantiate
     // anything which would require using the heap.  This function is the reason
     // ptr_map<> was used instead of ptr_set<>, which would have required
     // instantiating a ROW just to find a ROW.
-    LIB_TABLE*  cur = this;
-    ROWS_CITER  it;
+    const LIB_TABLE*  cur = this;
 
     do
     {
-        it = cur->rows.find( aLogicalName );
+        ROWS_CITER  it = cur->rows.find( aLogicalName );
 
         if( it != cur->rows.end() )
         {
@@ -192,9 +230,11 @@ const LIB_TABLE::ROW* LIB_TABLE::FindRow( const STRING& aLogicalName )
 
 bool LIB_TABLE::InsertRow( auto_ptr<ROW>& aRow, bool doReplace )
 {
-    ROWS_ITER it = rows.find( aRow->logicalName );
+    // this does not need to be super fast.
 
-    if( doReplace || it == rows.end() )
+    ROWS_CITER it = rows.find( aRow->logicalName );
+
+    if( it == rows.end() )
     {
         // be careful here, key is needed because aRow can be
         // release()ed before logicalName is captured.
@@ -203,6 +243,17 @@ bool LIB_TABLE::InsertRow( auto_ptr<ROW>& aRow, bool doReplace )
         return true;
     }
 
+    if( doReplace )
+    {
+        rows.erase( aRow->logicalName );
+
+        // be careful here, key is needed because aRow can be
+        // release()ed before logicalName is captured.
+        const STRING&   key = aRow->logicalName;
+        rows.insert( key, aRow );
+        return true;
+    }
+
     return false;
 }
 
@@ -218,9 +269,10 @@ void LIB_TABLE::Test()
     // To pass an empty string, we can pass " " to (options " ")
     SCH_LIB_TABLE_LEXER  slr(
         "(lib_table \n"
+        "  (lib (logical www)           (type http)     (full_uri http://kicad.org/libs)    (options \" \"))\n"
         "  (lib (logical meparts)       (type dir)      (full_uri /tmp/eeschema-lib)        (options \" \"))\n"
         "  (lib (logical old-project)   (type schematic)(full_uri /tmp/old-schematic.sch)   (options \" \"))\n"
-        "  (lib (logical www)           (type http)     (full_uri http://kicad.org/libs)    (options \" \"))\n",
+        ,
 
         wxT( "inline text" )        // source
         );
@@ -247,6 +299,7 @@ void LIB_TABLE::Test()
 
     STRING_FORMATTER    sf;
 
+    // format this whole table into sf, it will be sorted by logicalName.
     Format( &sf, 0 );
 
     printf( "test 'Parse() <-> Format()' round tripping:\n" );
@@ -264,6 +317,14 @@ void LIB_TABLE::Test()
     }
     else
         printf( "not found\n" );
+
+    printf( "\nlist of logical libraries:\n" );
+
+    STRINGS logNames = GetLogicalLibs();
+    for( STRINGS::const_iterator it = logNames.begin(); it!=logNames.end();  ++it )
+    {
+        printf( "logicalName: %s\n", it->c_str() );
+    }
 }
 
 

new/sch_lib_table.h

@@ -58,13 +58,6 @@ public:
 
     public:
 
-        /* was needed for ptr_set<> but not ptr_map<>
-        bool operator<( const ROW& other ) const
-        {
-            return logicalName < other.logicalName;
-        }
-        */
-
         /**
          * Function GetLogicalName
          * returns the logical name of this library table entry.
@@ -223,46 +216,54 @@ public:
      */
     PART* GetPart( const LPID& aLogicalPartID ) throw( IO_ERROR );
 
-
-#if 0   // moved here from LPID
     /**
-     * Function GetLogicalLibraries
-     * returns the logical library names, all of them that are in the
-     * library table.
+     * Function GetLogicalLibs
+     * returns the logical library names, all of them that are in pertinent to
+     * a lookup done on this LIB_TABLE.
      */
-    STRINGS GetLogicalLibraries();
+    STRINGS GetLogicalLibs();
+
+    //----<read accessors>----------------------------------------------------
+    // the returning of a const STRING* tells if not found, but might be too
+    // promiscuous?
 
     /**
-     * Function GetLibraryURI
+     * Function GetLibURI
      * returns the full library path from a logical library name.
      * @param aLogicalLibraryName is the short name for the library of interest.
-     * @param aSchematic provides access to the full library table inclusive
-     *  of the schematic contribution, or may be NULL to exclude the schematic rows.
+     * @return const STRING* - or NULL if not found.
      */
-    STRING  GetLibraryURI( const STRING& aLogicalLibraryName,
-                        SCHEMATIC* aSchematic=NULL ) const;
+    const STRING* GetLibURI( const STRING& aLogicalLibraryName ) const
+    {
+        const ROW* row = FindRow( aLogicalLibraryName );
+        return row ? &row->fullURI : 0;
+    }
 
     /**
-     * Function GetLibraryType
+     * Function GetLibType
      * returns the type of a logical library.
      * @param aLogicalLibraryName is the short name for the library of interest.
-     * @param aSchematic provides access to the full library table inclusive
-     *  of the schematic contribution, or may be NULL to exclude the schematic rows.
+     * @return const STRING* - or NULL if not found.
      */
-    STRING GetLibraryType( const STRING& aLogicalLibraryName,
-                        SCHEMATIC* aSchematic=NULL ) const;
+    const STRING* GetLibType( const STRING& aLogicalLibraryName ) const
+    {
+        const ROW* row = FindRow( aLogicalLibraryName );
+        return row ? &row->libType : 0;
+    }
 
     /**
-     * Function GetOptions
+     * Function GetLibOptions
      * returns the options string for \a aLogicalLibraryName.
      * @param aLogicalLibraryName is the short name for the library of interest.
-     * @param aSchematic provides access to the full library table inclusive
-     *  of the schematic contribution, or may be NULL to exclude the schematic rows.
+     * @return const STRING* - or NULL if not found.
      */
-    STRING GetOptions( const STRING& aLogicalLibraryName,
-                        SCHEMATIC* aSchematic=NULL ) const;
-#endif
+    const STRING* GetLibOptions( const STRING& aLogicalLibraryName ) const
+    {
+        const ROW* row = FindRow( aLogicalLibraryName );
+        return row ? &row->options : 0;
+    }
 
+    //----</read accessors>---------------------------------------------------
 
 #if defined(DEBUG)
     /// implement the tests in here so we can honor the priviledge levels of the
@@ -276,9 +277,10 @@ protected:  // only a table editor can use these
      * Function InsertRow
      * adds aRow if it does not already exist or if doReplace is true.  If doReplace
      * is not true and the key for aRow already exists, the function fails and returns false.
+     * The key for the table is the logicalName, and all in this table must be unique.
      * @param aRow is the new row to insert, or to forcibly add if doReplace is true.
-     * @param doReplace if true, means insert regardless if aRow's key already exists.  If false, then fail
-     *  if the key already exists.
+     * @param doReplace if true, means insert regardless of whether aRow's key already
+     *  exists.  If false, then fail if the key already exists.
      * @return bool - true if the operation succeeded.
      */
     bool InsertRow( std::auto_ptr<ROW>& aRow, bool doReplace = false );
@@ -287,7 +289,7 @@ protected:  // only a table editor can use these
      * Function FindRow
      * returns a ROW* if aLogicalName is found in this table or in fallBack, else NULL.
      */
-    const ROW* FindRow( const STRING& aLogicalName );
+    const ROW* FindRow( const STRING& aLogicalName ) const;
 
 private: