[glom] Import: Cleanup, added comments

[Michael Hasselmann <mikhas src gnome org>]

commit b9d81b6af4678b47fc43f914c9b2bb9628293023
Author: Michael Hasselmann <michaelh openismus com>
Date:   Fri Oct 2 12:23:00 2009 +0200

    Import: Cleanup, added comments

 ChangeLog                                     |    4 ++++
 glom/import_csv/csv_parser.h                  |   25 +++++++++++++++----------
 glom/import_csv/dialog_import_csv.cc          |   25 ++++++++-----------------
 glom/import_csv/dialog_import_csv.h           |    8 ++++----
 glom/import_csv/dialog_import_csv_progress.cc |    4 +++-
 5 files changed, 34 insertions(+), 32 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index de1c401..b6b841a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2009-10-02  Michael Hasselmann  <michaelh openismus com>
+
+	Import: Cleanup, added comments.
+
 2009-10-01  Michael Hasselmann  <michaelh openismus com>
 
 	Import: Made import working again, changed import logic
diff --git a/glom/import_csv/csv_parser.h b/glom/import_csv/csv_parser.h
index 186fb5a..c90e9d0 100644
--- a/glom/import_csv/csv_parser.h
+++ b/glom/import_csv/csv_parser.h
@@ -36,6 +36,13 @@ namespace Glom
 // We use the low-level Glib::IConv routines to progressively convert the
 // input data in an idle handler.
 
+// TODO: Kill the caching of complete files within m_rows (too costly for big
+// files) and instead only fill it with n rows (and stop parsing until the row
+// buffer has some room again). For accessing parsed rows, one could have two
+// methods: fetch_next_row/take_next_row. The first would use an index, whereas
+// the latter would return the first row in the row buffer and delete it from
+// the buffer.
+
 /** Parses .csv (comma-separated values) text files.
  * See http://en.wikipedia.org/wiki/Comma-separated_values for the file format.
  *
@@ -66,23 +73,20 @@ public:
   /// Get the current state of the parser.
   State get_state() const;
 
-/*
-  /// Get the number of rows parsed so far.
-  guint get_rows_count() const;
-*/
-
   bool get_rows_empty() const;
 
-/*
-  /// Get the number of columns of data in this row.
-  guint get_cols_count(guint row_number) const;
-*/
-
   // The nasty reference return is for performance.
   const Glib::ustring& get_data(guint row, guint col);
 
+  /**  Fetches the next row from the parser's cache. It will block until enough
+    *  data was parsed to return a row. An empty row indicates the last row was
+    *  fetched.
+    */
+  // TODO: Fix to cope with valid empty rows in between, without requiring the
+  // client of this method to check for the parser's state/cache size.
   const type_row_strings fetch_next_row();
 
+  /// Resets the internal row index used for fetch_next_row().
   void reset_row_index();
 
   // Signals:
@@ -186,6 +190,7 @@ private:
   // Parsed data:
   type_rows m_rows;
 
+  // Indicates which row to fetch next:
   guint m_row_index;
 
   type_signal_file_read_error m_signal_file_read_error;
diff --git a/glom/import_csv/dialog_import_csv.cc b/glom/import_csv/dialog_import_csv.cc
index cd2dedd..66a6cf1 100644
--- a/glom/import_csv/dialog_import_csv.cc
+++ b/glom/import_csv/dialog_import_csv.cc
@@ -155,9 +155,9 @@ Glib::ustring Dialog_Import_CSV::get_target_table_name() const
   return m_target_table->get_text();
 }
 
-const Glib::ustring& Dialog_Import_CSV::get_filename() const
+const Glib::ustring& Dialog_Import_CSV::get_file_uri() const
 {
-  return m_filename;
+  return m_file_uri;
 }
 
 void Dialog_Import_CSV::import(const Glib::ustring& uri, const Glib::ustring& into_table)
@@ -208,17 +208,11 @@ void Dialog_Import_CSV::import(const Glib::ustring& uri, const Glib::ustring& in
     m_field_model_sorted = Gtk::TreeModelSort::create(m_field_model);
     m_field_model_sorted->set_sort_column(m_field_columns.m_col_field_name, Gtk::SORT_ASCENDING);
 
-    m_filename = uri;
+    m_file_uri = uri;
     m_parser->set_file_and_start_parsing(uri);
   }
 }
 
-// TODO:remove me.
-guint Dialog_Import_CSV::get_row_count() const
-{
-  return -1;
-}
-
 guint Dialog_Import_CSV::get_column_count() const
 {
   return m_cols_count;
@@ -252,7 +246,7 @@ void Dialog_Import_CSV::clear()
   m_field_model.reset();
   m_field_model_sorted.reset();
   m_fields.clear();
-  m_filename.clear();
+  m_file_uri.clear();
   m_parser->clear();
   //m_parser.reset(0);
   m_encoding_info->set_text("");
@@ -304,7 +298,7 @@ void Dialog_Import_CSV::on_combo_encoding_changed()
 
   // Parse from beginning with new encoding:
   m_parser->set_encoding(get_current_encoding());
-  m_parser->set_file_and_start_parsing(m_filename);
+  m_parser->set_file_and_start_parsing(m_file_uri);
 }
 
 void Dialog_Import_CSV::on_first_line_as_title_toggled()
@@ -467,9 +461,6 @@ void Dialog_Import_CSV::on_parser_encoding_error()
   }
 }
 
-/*
- * No, this is wrong. Creating the tree model and handling a line from the CSV file are two separate steps. Proposal: Construct tree model *after* parsing, using row[0].
- */
 void Dialog_Import_CSV::on_parser_line_scanned(CsvParser::type_row_strings row, unsigned int row_number)
 {
   // This is the first line read if there is no model yet:
@@ -497,7 +488,7 @@ void Dialog_Import_CSV::on_parser_line_scanned(CsvParser::type_row_strings row,
   }
 }
 
-void Dialog_Import_CSV::setup_sample_model(CsvParser::type_row_strings& row)
+void Dialog_Import_CSV::setup_sample_model(const CsvParser::type_row_strings& row)
 {
   m_sample_model = Gtk::ListStore::create(m_sample_columns);
   m_sample_view->set_model(m_sample_model);
@@ -734,8 +725,8 @@ void Dialog_Import_CSV::validate_primary_key()
 
 void Dialog_Import_CSV::on_parser_file_read_error(const Glib::ustring& error_message)
 {
-  show_error_dialog(_("Could Not Open file"), 
-    Glib::ustring::compose(_("The file at \"%1\" could not be opened: %2"), m_filename, error_message) );
+  show_error_dialog(_("Could Not Open file"),
+    Glib::ustring::compose(_("The file at \"%1\" could not be opened: %2"), Glib::filename_from_uri(m_file_uri), error_message) );
 }
 
 void Dialog_Import_CSV::on_parser_have_display_name(const Glib::ustring& display_name)
diff --git a/glom/import_csv/dialog_import_csv.h b/glom/import_csv/dialog_import_csv.h
index da724fb..bc02de9 100644
--- a/glom/import_csv/dialog_import_csv.h
+++ b/glom/import_csv/dialog_import_csv.h
@@ -50,13 +50,13 @@ public:
 
   CsvParser::State get_parser_state() const;
   Glib::ustring get_target_table_name() const;
-  const Glib::ustring& get_filename() const;
+  const Glib::ustring& get_file_uri() const;
 
-  unsigned int get_row_count() const;
   unsigned int get_column_count() const;
   sharedptr<const Field> get_field_for_column(unsigned int col) const;
   const Glib::ustring& get_data(unsigned int row, unsigned int col);
 
+  // TODO: perhaps it would be safer to just wrap the needed parser API here.
   CsvParser& get_parser();
 
   typedef sigc::signal<void> type_signal_state_changed;
@@ -73,7 +73,7 @@ private:
   Glib::ustring get_current_encoding() const;
   void begin_parse();
 
-  void setup_sample_model(CsvParser::type_row_strings& row);
+  void setup_sample_model(const CsvParser::type_row_strings& row);
   Gtk::TreeViewColumn* create_sample_column(const Glib::ustring& title, guint index);
   Gtk::CellRendererCombo* create_sample_cell(guint index);
 
@@ -143,7 +143,7 @@ private:
   Gtk::Label* m_advice_label;
   Gtk::Label* m_error_label;
 
-  Glib::ustring m_filename;
+  Glib::ustring m_file_uri;
 
   // Index into the ENCODINGS array (see dialog_import_csv.cc) for the
   // encoding that we currently try to read the data with, or -1 if
diff --git a/glom/import_csv/dialog_import_csv_progress.cc b/glom/import_csv/dialog_import_csv_progress.cc
index bcc818f..6a6a224 100644
--- a/glom/import_csv/dialog_import_csv_progress.cc
+++ b/glom/import_csv/dialog_import_csv_progress.cc
@@ -63,7 +63,7 @@ void Dialog_Import_CSV_Progress::import(Dialog_Import_CSV& data_source)
     // Wait for the parsing to finish. We do not start importing before the file has been
     // parsed completely since we would not to rollback our changes in case of a
     // parsing error.
-    m_progress_bar->set_text(Glib::ustring::compose(_("Parsing CSV file %1"), data_source.get_filename()));
+    m_progress_bar->set_text(Glib::ustring::compose(_("Parsing CSV file %1"), Glib::filename_from_uri(data_source.get_file_uri())));
     m_ready_connection = data_source.signal_state_changed().connect(sigc::mem_fun(*this, &Dialog_Import_CSV_Progress::on_state_changed));
     break;
   case CsvParser::STATE_PARSED:
@@ -133,6 +133,8 @@ bool Dialog_Import_CSV_Progress::on_idle_import()
 
   const CsvParser::type_row_strings row = m_data_source->get_parser().fetch_next_row();
 
+  // TODO: Perhaps abort on 0 == row instead, so that we do not stop import on
+  // empty rows that are in between other rows.
   if(row.empty())
   {
     // Don't do the response immediately, so the user has a chance to read the