Index: main/trunk/greenstone2/perllib/gssql.pm
===================================================================
--- main/trunk/greenstone2/perllib/gssql.pm	(revision 32542)
+++ main/trunk/greenstone2/perllib/gssql.pm	(revision 32543)
@@ -212,5 +212,5 @@
 # GreenstoneSQLPlugout, this method will not attempt to create the requested db (nor its tables)
 # TODO: GS SQLPlugin is called before GS SQLPlugout and attempts to use_db() - called in plugin's
-# init() method. This will fail if the db does not exist. Ideally want our plugin only called
+# init() method. This will fail if the db does not exist. Ideally want the gssqlplugin only called
 # during buildcol.pl
 sub use_db {
@@ -282,5 +282,5 @@
 
 # "IF EXISTS is used to prevent an error from occurring if the database does not exist. ... DROP DATABASE returns the number of tables that were removed. The DROP DATABASE statement removes from the given database directory those files and directories that MySQL itself may create during normal operation.Jun 20, 2012"
-#MySQL 8.0 Reference Manual :: 13.1.22 DROP DATABASE Syntax
+# MySQL 8.0 Reference Manual :: 13.1.22 DROP DATABASE Syntax
 # https://dev.mysql.com/doc/en/drop-database.html
 sub delete_collection_tables {
@@ -432,5 +432,5 @@
 # see https://www.perlmonks.org/bare/?node=DBI%20Recipes
 #    The page further has a table_exists function that could work with proper comparison
-# TODO: Couldn't get the first solution at https://www.perlmonks.org/bare/?node_id=500050 to work though
+# TODO Q: Couldn't get the first solution at https://www.perlmonks.org/bare/?node_id=500050 to work though
 sub table_exists {
     my $self = shift (@_);
Index: main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32542)
+++ main/trunk/greenstone2/perllib/plugins/GreenstoneSQLPlugin.pm	(revision 32543)
@@ -40,10 +40,11 @@
 # TODO:
 # - Run TODOs here, in Plugout and in gssql.pm by Dr Bainbridge.
-# Ask about docsql naming convention adopted to identify OID. Better way?
+# + Ask about docsql naming convention adopted to identify OID. Better way?
 # collection names -> table names: it seems hyphens not allowed. Changed to underscores.
 # + Startup parameters (except removeold/build_mode)
+# - how do we detect we're to do removeold during plugout in import.pl phase
 # - incremental building: where do we need to add code to delete rows from our sql table after
 # incrementally importing a coll with fewer docs (for instance)? What about deleted/modified meta?
-# - Courier documents in lucene-sql collection: character (degree symbol) not preserved. Is this because we encode in utf8 when putting into db and reading back in?
+# - "Courier" demo documents in lucene-sql collection: character (degree symbol) not preserved in title. Is this because we encode in utf8 when putting into db and reading back in?
 # - Have not yet tested writing out just meta or just fulltxt to sql db and reading just that 
 # back in from the sql db while the remainder is to be read back in from the docsql .xml files.
@@ -51,4 +52,11 @@
 # (sections) which in this case had made it easy to reconstruct the doc_obj in memory in the correct order
 
+# TODO: deal with incremental vs removeold. If docs removed from import folder, then import step
+# won't delete it from archives but buildcol step will. Need to implement this with this database plugin or wherever the actual flow is
+
+# TODO: Add public instructions on using this plugin and its plugout: start with installing mysql binary, changing pwd, running the server (and the client against it for checking, basic cmds like create and drop). Then discuss db name, table names (per coll), db cols and col types, and how the plugout and plugin work.
+# Discuss the plugin/plugout parameters.
+
+
 # GreenstoneSQLPlugin inherits from GreenstoneXMLPlugin so that it if meta or fulltext
 # is still written out to doc.xml (docsql .xml), that will be processed as usual,
@@ -56,13 +64,4 @@
 # is written out by GreenstoneSQLPlugout into the SQL db).
 
-# TODO:
-# no more docoid in docsql .xml filename, set OID as attribute of root element inside docsql.xml file instead
-# and parse it out
-
-# TODO: deal with incremental vs removeold. If docs removed from import folder, then import step
-# won't delete it from archives but buildcol step will. Need to implement this with this database plugin or wherever the actual flow is
-
-# TODO: Add public instructions on using this plugin and its plugout: start with installing mysql binary, changing pwd, running the server (and the client against it for checking, basic cmds like create and drop). Then discuss db name, table names (per coll), db cols and col types, and how the plugout and plugin work.
-# Discuss the plugin/plugout parameters.
 
 sub BEGIN {
@@ -201,24 +200,14 @@
 
     my $oid = $self->{'doc_oid'}; # we stored current doc's OID during sub xml_start_tag()
-    print $outhandle "==== OID of document (meta|text) to be read in from DB: $oid\n"
+    print $outhandle "++++ OID of document (meta|text) to be read in from DB: $oid\n"
 	if $self->{'verbosity'} > 1;
-
     
     # For now, we have access to doc_obj (until just before super::close_document() terminates)
-    
-    $self->{'doc_obj'}->set_OID($oid); # complex method. Is this necessary, since we just want to write meta and txt for the docobj to index?
-    
-    # checking that complicated looking method set_OID() hasn't modified oid
-    if($oid ne $self->{'doc_obj'}->get_OID()) {
-	print STDERR "@@@@ WARNING: OID after setting on doc_obj = " . $self->{'doc_obj'}->get_OID() . " and is not the same as original OID $oid from docsqloid.xml filename\n";
-    }
-
-    
-    # TODO: This function is called on a per doc.xml file basis
-    # but we can process all docs of a collection in one go when dealing with the SQL tables for
-    # the collection. How and where should we read in the collection tables then?
-    # TODO: Perhaps MySQLPlugout could write out a token file (.gssql) into archives during import.pl
-    # and if that file is detected, then MySQLPlugin::read() is passed in that file during
-    # buildcol.pl. And that file will trigger reading the 2 tables for the collection???
+
+    # no need to call $self->{'doc_obj'}->set_OID($oid);
+    # because either the OID is stored in the SQL db as meta 'Identifier' alongside other metadata
+    # or it's stored in the doc.xml as metadata 'Identifier' alongside other metadata
+    # Either way, Identifier meta will be read into the docobj automatically with other meta.
+
     my $proc_mode = $self->{'process_mode'};
     if($proc_mode eq "all" || $proc_mode eq "meta_only") {
@@ -226,5 +215,6 @@
 
 	my $sth = $gs_sql->select_from_metatable_matching_docid($oid);	
-	print $outhandle "### stmt: ".$sth->{'Statement'}."\n" if $self->{'verbosity'} > 1;
+	print $outhandle "### SQL select stmt: ".$sth->{'Statement'}."\n"
+	    if $self->{'verbosity'} > 1;
 
 	print $outhandle "----------SQL DB contains meta-----------\n" if $self->{'verbosity'} > 1;
@@ -281,6 +271,9 @@
 
 
-# TODO: only want to work with sql db if buildcol.pl. Unfortunately, also runs on import.pl
-# call init() not begin() because there can be multiple plugin passes
+# TODO: only want to work with sql db if buildcol.pl. Unfortunately, also runs on import.pl.
+# During import, the GS SQL Plugin is called before the GS SQL Plugout with undesirable side
+# effect that if the db doesn't exist, gssql::use_db() fails, as it won't create db.
+
+# Call init() not begin() because there can be multiple plugin passes
 # and init() should be called before all passes:
 # one for doc level and another for section level indexing
Index: main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm	(revision 32542)
+++ main/trunk/greenstone2/perllib/plugouts/GreenstoneSQLPlugout.pm	(revision 32543)
@@ -40,8 +40,9 @@
 
 # TODO: SIGTERM rollback and disconnect?
-# TODO Q: what about verbosity for debugging
+# TODO Q: what about verbosity for debugging, instead of current situation of printing out upon debug set at the expense of writing to db
 # TODO Q: introduced site_name param to plugins and plugouts. Did I do it right? And should they have hiddengli = "yes"
 
-# this plugout does not output xml to a file, but outputs rows into a mysql table
+# this plugout does not output the metadata and/or fulltxt xml to a file,
+# but outputs rows into a mysql table for metadata and/or a table for fulltxt
 sub BEGIN {
     @GreenstoneSQLPlugout::ISA = ('GreenstoneXMLPlugout');
@@ -56,6 +57,4 @@
 
 # TODO Q: what is "group" in GreenstoneXMLPlugout?
-# TODO Q: site_name only exists for GS3. What about GS2?
-
 
 my $process_mode_list =
@@ -141,5 +140,5 @@
     #print STDERR "@@@@ proc_mode: " . $self->{'process_mode'} . "\n";
     
-    ########### TODO: these should be set from cmdline/GLI options to plugout #########
+    ########### TODO: deal with build mode #########
 
     $self->{'build_mode'} = "removeold";
@@ -236,5 +235,6 @@
 }
  
-# TODO: check arc-inf.db for whether each entry is to be deleted/indexed/reindexed/been indexed
+# TODO: check arc-inf.db for whether each entry is to be deleted/indexed/reindexed/been indexed?
+# That's only for indexing, not for this step which only generates the content in archives dir
 sub saveas {
     my $self = shift (@_);
@@ -253,14 +253,13 @@
 
     # TODO: also set debugging in begin()? Then stmts creating db and tables also sent to debug out and not executed
-
-    # TODO: remove unused old_unused_saveas from GreenstoneXMLPlugout
-    
-    
-    # 2. overriding saving behaviour to do what the superclass does PLUS saving to sql db    
-
-    #NOTE: if proc_mode == all, then "breadcrumbs" go into both meta and txt elements of doc.xml:
-    # statements pointing viewer to the sql db for contents
+   
+    
+    # 2. overriding saving behaviour to do what the superclass does (writing out doc.xml files,
+    # under new name of docsql.xml, with breadcrumbs pointing to sql db) PLUS saving to sql db    
+
+    # NOTE: if proc_mode == all, then "breadcrumbs" (statements pointing viewer to the sql db
+    # for contents) go into both meta and txt elements of doc.xml (docsql.xml specifically): 
 	
-    # write the INVERSE into doc.xml as to what is written to the db    
+    # write the INVERSE into doc.xml as to what is written to the SQL db    
     my $docxml_output_options = { 'output' => docprint::OUTPUT_NONE };
     if($proc_mode eq "meta_only" ) { # since only meta to go into MySQL db, text will go into docxml
@@ -272,5 +271,5 @@
     # now we've prepared to write out whatever is meant to go into docxml
     # and can do actual the steps superclass GreenstoneXMLPlugout carries out to write out docxml
-    # So: write out the doc xml file for the current document
+    # So: write out the doc xml file, "docsql.xml", for the current document
     my $section_text = &docprint::get_section_xml($doc_obj, $docxml_output_options);
     print $docxml_outhandler $section_text;    
Index: main/trunk/greenstone2/perllib/plugouts/GreenstoneXMLPlugout.pm
===================================================================
--- main/trunk/greenstone2/perllib/plugouts/GreenstoneXMLPlugout.pm	(revision 32542)
+++ main/trunk/greenstone2/perllib/plugouts/GreenstoneXMLPlugout.pm	(revision 32543)
@@ -76,70 +76,4 @@
 }
 
-sub old_unused_saveas {
-    my $self = shift (@_);
-    my ($doc_obj, $doc_dir) = @_;
-    my $outhandler;
-    my $output_file;
-    if ($self->{'debug'}) {
-	$outhandler = STDOUT;
-    }
-    else {
-	   
-	$self->process_assoc_files($doc_obj, $doc_dir, '');
-	$self->process_metafiles_metadata ($doc_obj);
-	
-	# open up the outhandler    
-	if ($self->is_group() && !$self->{'new_doc_dir'}) { 
-	    # we already have a handle open ??
-	    $outhandler = $self->{'group_outhandler'};
-	} else {
-	    $output_file = &FileUtils::filenameConcatenate(
-		$self->{'output_dir'}, $doc_dir, $self->get_doc_xml_filename($doc_obj));
-	    # open the new handle
-	    $self->open_xslt_pipe($output_file, $self->{'xslt_file'});
-
-	    if (defined $self->{'xslt_writer'}){
-		$outhandler = $self->{'xslt_writer'};
-	    }
-	    else{
-		$outhandler = $self->get_output_handler($output_file);
-	    }
-	    
-	    if ($self->is_group()) {
-		$self->{'group_outhandler'} = $outhandler;
-	    }
-	}
-    } # else not debug
-    binmode($outhandler,":utf8");
-
-    # only output the header if we have started a new doc
-    if (!$self->is_group() || $self->{'new_doc_dir'}) {
-	$self->output_xml_header($outhandler);
-    }
-
-    my $section_text = &docprint::get_section_xml($doc_obj);
-    print $outhandler $section_text;
- 
-    # only output the footer if we are not doing group stuff. The group file will be finished in close_group_output
-    if (!$self->is_group()) {
-	$self->output_xml_footer($outhandler);
-    }
-
-    # close off the output - in a group process situation, this will be done by close_group_output
-    if (!$self->is_group() && !$self->{'debug'}) {
-	if (defined $self->{'xslt_writer'}){     
-	    $self->close_xslt_pipe(); 
-	}
-	else {
-	    &FileUtils::closeFileHandle($output_file, \$outhandler) if defined $output_file;
-	}
-    }
-    $self->{'short_doc_file'} = &FileUtils::filenameConcatenate(
-	$doc_dir, $self->get_doc_xml_filename($doc_obj));  
-    
-    $self->store_output_info_reference($doc_obj);
-    
-}
-
 # can be overridden in subclasses, for instance by GreenstoneSQLPlugout, to produce a different filename
 # like docsql.xml
