Move column parsing earlier up the chain of events

In this form not all data needs reparsing when a single column
changes. The drawback is it makes the code slightly more complicated
because accounts can only be verified after the account match page
has completed, while most columns are set on the preview page.

src/import-export/csv-imp/assistant-csv-trans-import.cpp

@@ -1309,12 +1309,7 @@ void CsvImpTransAssist::preview_row_sel_update ()
  */
 void CsvImpTransAssist::preview_refresh_table ()
 {
-    auto save_skip_errors = tx_imp->skip_err_lines();
-    tx_imp->update_skipped_lines (boost::none, boost::none,
-        boost::none, false);
     preview_validate_settings ();
-    tx_imp->update_skipped_lines (boost::none, boost::none,
-        boost::none, save_skip_errors);
 
     /* ncols is the number of columns in the file data. */
     auto column_types = tx_imp->column_types();
@@ -1766,6 +1761,8 @@ CsvImpTransAssist::assist_preview_page_prepare ()
     g_signal_connect (G_OBJECT(treeview), "size-allocate",
                      G_CALLBACK(csv_tximp_preview_treeview_resized_cb), (gpointer)this);
 
+    tx_imp->req_mapped_accts (false);
+
     /* Disable the Forward Assistant Button */
     gtk_assistant_set_page_complete (csv_imp_asst, preview_page, false);
 
@@ -1776,6 +1773,8 @@ CsvImpTransAssist::assist_preview_page_prepare ()
 void
 CsvImpTransAssist::assist_account_match_page_prepare ()
 {
+    tx_imp->req_mapped_accts(true);
+
     // Load the account strings into the store
     acct_match_set_accounts ();
 
@@ -1804,6 +1803,36 @@ CsvImpTransAssist::assist_doc_page_prepare ()
     /* Block going back */
     gtk_assistant_commit (csv_imp_asst);
 
+    /* At this stage in the assistant each account should be mapped so
+     * complete the split properties with this information. If this triggers
+     * an exception it indicates a logic error in the code.
+     */
+    try
+    {
+        auto col_types = tx_imp->column_types();
+        auto acct_col = std::find (col_types.begin(),
+                col_types.end(), GncTransPropType::ACCOUNT);
+        if (acct_col != col_types.end())
+            tx_imp->set_column_type (acct_col - col_types.begin(),
+                    GncTransPropType::ACCOUNT, true);
+        acct_col = std::find (col_types.begin(),
+                col_types.end(), GncTransPropType::TACCOUNT);
+        if (acct_col != col_types.end())
+            tx_imp->set_column_type (acct_col - col_types.begin(),
+                    GncTransPropType::TACCOUNT, true);
+    }
+    catch (const std::invalid_argument& err)
+    {
+        /* Oops! This shouldn't happen when using the import assistant !
+         * Inform the user and go back to the preview page.
+         */
+        gnc_error_dialog (GTK_WIDGET(csv_imp_asst),
+            _("An unexpected error has occurred while mapping accounts. Please report this as a bug.\n\n"
+              "Error message:\n%s"), err.what());
+        gtk_assistant_set_current_page (csv_imp_asst, 2);
+
+    }
+
     /* Before creating transactions, if this is a new book, let user specify
      * book options, since they affect how transactions are created */
     if (new_book)
@@ -1832,7 +1861,7 @@ CsvImpTransAssist::assist_match_page_prepare ()
          * Inform the user and go back to the preview page.
          */
         gnc_error_dialog (GTK_WIDGET(csv_imp_asst),
-            _("An unexpected error has occurred. Please report this as a bug.\n\n"
+            _("An unexpected error has occurred while creating transactions. Please report this as a bug.\n\n"
               "Error message:\n%s"), err.what());
         gtk_assistant_set_current_page (csv_imp_asst, 2);
     }

src/import-export/csv-imp/gnc-trans-props.cpp

@@ -161,13 +161,13 @@ time64 parse_date (const std::string &date_str, int format)
     boost::regex r(date_regex[format]);
     boost::smatch what;
     if(!boost::regex_search(date_str, what, r))
-        throw std::invalid_argument ("String doesn't appear to be formatted as a date.");  // regex didn't find a match
+        throw std::invalid_argument (_("Value can't be parsed into a date using the selected date format."));  // regex didn't find a match
 
     // Attention: different behavior from 2.6.x series !
     // If date format without year was selected, the match
     // should NOT have found a year.
     if ((format >= 3) && (what.length("YEAR") != 0))
-        throw std::invalid_argument ("String appears to contain a year while the selected format forbids this.");
+        throw std::invalid_argument (_("Value appears to contain a year while the selected format forbids this."));
 
     auto day = std::stoi (what.str("DAY"));
     auto month = std::stoi (what.str("MONTH"));
@@ -210,7 +210,7 @@ gnc_numeric parse_amount (const std::string &str, int currency_format)
 {
     /* If a cell is empty or just spaces return invalid amount */
     if(!boost::regex_search(str, boost::regex("[0-9]")))
-        throw std::invalid_argument ("String doesn't appear to contain a valid number.");
+        throw std::invalid_argument (_("Value doesn't appear to contain a valid number."));
 
     auto expr = boost::make_u32regex("[[:Sc:]]");
     std::string str_no_symbols = boost::u32regex_replace(str, expr, "");
@@ -223,17 +223,17 @@ gnc_numeric parse_amount (const std::string &str, int currency_format)
     case 0:
         /* Currency locale */
         if (!(xaccParseAmount (str_no_symbols.c_str(), TRUE, &val, &endptr)))
-            throw std::invalid_argument ("String can't be parsed into a number using the selected currency format.");
+            throw std::invalid_argument (_("Value can't be parsed into a number using the selected currency format."));
         break;
     case 1:
         /* Currency decimal period */
         if (!(xaccParseAmountExtended (str_no_symbols.c_str(), TRUE, '-', '.', ',', "\003\003", "$+", &val, &endptr)))
-            throw std::invalid_argument ("String can't be parsed into a number using the selected currency format.");
+            throw std::invalid_argument (_("Value can't be parsed into a number using the selected currency format."));
         break;
     case 2:
         /* Currency decimal comma */
         if (!(xaccParseAmountExtended (str_no_symbols.c_str(), TRUE, '-', ',', '.', "\003\003", "$+", &val, &endptr)))
-            throw std::invalid_argument ("String can't be parsed into a number using the selected currency format.");
+            throw std::invalid_argument (_("Value can't be parsed into a number using the selected currency format."));
         break;
     }
 
@@ -253,7 +253,7 @@ static char parse_reconciled (const std::string& reconcile)
     else if (g_strcmp0 (reconcile.c_str(), _("v")) == 0) // Voided will be handled at the transaction level
         return NREC;                                      // so return not reconciled here
     else
-        throw std::invalid_argument ("String can't be parsed into a valid reconcile state.");
+        throw std::invalid_argument (_("Value can't be parsed into a valid reconcile state."));
 }
 
 gnc_commodity* parse_commodity (const std::string& comm_str)
@@ -291,62 +291,84 @@ gnc_commodity* parse_commodity (const std::string& comm_str)
     }
 
     if (!comm)
-        throw std::invalid_argument ("String can't be parsed into a valid commodity.");
+        throw std::invalid_argument (_("Value can't be parsed into a valid commodity."));
     else
         return comm;
 }
 
 void GncPreTrans::set (GncTransPropType prop_type, const std::string& value)
 {
-    gnc_commodity *comm = nullptr;
+    try
-    switch (prop_type)
     {
-        case GncTransPropType::UNIQUE_ID:
+        // Drop any existing error for the prop_type we're about to set
-            m_differ = boost::none;
+        m_errors.erase(prop_type);
-            if (!value.empty())
-                m_differ = value;
-            break;
 
-        case GncTransPropType::DATE:
+        gnc_commodity *comm = nullptr;
-            m_date = boost::none;
+        switch (prop_type)
-            m_date = parse_date (value, m_date_format); // Throws if parsing fails
+        {
-            break;
+            case GncTransPropType::UNIQUE_ID:
+                m_differ = boost::none;
+                if (!value.empty())
+                    m_differ = value;
+                break;
 
-        case GncTransPropType::NUM:
+            case GncTransPropType::DATE:
-            m_num = boost::none;
+                m_date = boost::none;
-            if (!value.empty())
+                m_date = parse_date (value, m_date_format); // Throws if parsing fails
-                m_num = value;
+                break;
-            break;
 
-        case GncTransPropType::DESCRIPTION:
+            case GncTransPropType::NUM:
-            m_desc = boost::none;
+                m_num = boost::none;
-            if (!value.empty())
+                if (!value.empty())
-                m_desc = value;
+                    m_num = value;
-            break;
+                break;
 
-        case GncTransPropType::NOTES:
+            case GncTransPropType::DESCRIPTION:
-            m_notes = boost::none;
+                m_desc = boost::none;
-            if (!value.empty())
+                if (!value.empty())
-                m_notes = value;
+                    m_desc = value;
-            break;
+                break;
 
-        case GncTransPropType::COMMODITY:
+            case GncTransPropType::NOTES:
-            m_commodity = boost::none;
+                m_notes = boost::none;
-            comm = parse_commodity (value); // Throws if parsing fails
+                if (!value.empty())
-            if (comm)
+                    m_notes = value;
-                m_commodity = comm;
+                break;
-            break;
 
-        case GncTransPropType::VOID_REASON:
+            case GncTransPropType::COMMODITY:
-            m_void_reason = boost::none;
+                m_commodity = boost::none;
-            if (!value.empty())
+                comm = parse_commodity (value); // Throws if parsing fails
-                m_void_reason = value;
+                if (comm)
-            break;
+                    m_commodity = comm;
+                break;
 
-        default:
+            case GncTransPropType::VOID_REASON:
-            /* Issue a warning for all other prop_types. */
+                m_void_reason = boost::none;
-            PWARN ("%d is an invalid property for a transaction", static_cast<int>(prop_type));
+                if (!value.empty())
-            break;
+                    m_void_reason = value;
+                break;
+
+            default:
+                /* Issue a warning for all other prop_types. */
+                PWARN ("%d is an invalid property for a transaction", static_cast<int>(prop_type));
+                break;
+        }
+    }
+    catch (const std::invalid_argument& e)
+    {
+        auto err_str = std::string(_(gnc_csv_col_type_strs[prop_type])) +
+                       std::string(_(" could not be understood.\n")) +
+                       e.what();
+        m_errors.emplace(prop_type, err_str);
+        throw std::invalid_argument (err_str);
+    }
+    catch (const std::out_of_range& e)
+    {
+        auto err_str = std::string(_(gnc_csv_col_type_strs[prop_type])) +
+                       std::string(_(" could not be understood.\n")) +
+                       e.what();
+        m_errors.emplace(prop_type, err_str);
+        throw std::invalid_argument (err_str);
     }
 
 }
@@ -360,7 +382,8 @@ void GncPreTrans::reset (GncTransPropType prop_type)
     catch (...)
     {
         // Set with an empty string will effectively clear the property
-        // but also throw in many cases. For a reset this is fine, so catch it here.
+        // but can also set an error for the property. Clear that error here.
+        m_errors.erase(prop_type);
     }
 }
 
@@ -418,98 +441,151 @@ bool GncPreTrans::is_part_of (std::shared_ptr<GncPreTrans> parent)
             (!m_desc || m_desc == parent->m_desc) &&
             (!m_notes || m_notes == parent->m_notes) &&
             (!m_commodity || m_commodity == parent->m_commodity) &&
-            (!m_void_reason || m_void_reason == parent->m_void_reason);
+            (!m_void_reason || m_void_reason == parent->m_void_reason) &&
+            parent->m_errors.empty(); // A GncPreTrans with errors can never be a parent
+}
+
+/* Declare two translatable error strings here as they will be used in several places */
+const char *bad_acct = N_("Account value can't be mapped back to an account.");
+const char *bad_tacct = N_("Transfer account value can't be mapped back to an account.");
+
+static std::string gen_err_str (std::map<GncTransPropType, std::string>& errors,
+        bool check_accts_mapped = false)
+{
+    auto full_error = std::string();
+    for (auto error : errors)
+    {
+        auto err_str = error.second;
+        if (!check_accts_mapped &&
+                ((err_str.find (_(bad_acct)) != std::string::npos) ||
+                 (err_str.find (_(bad_tacct)) != std::string::npos)))
+            continue;
+        full_error += (full_error.empty() ? "" : "\n") + error.second;
+    }
+
+    return full_error;
+}
+
+std::string GncPreTrans::errors ()
+{
+    return gen_err_str (m_errors);
 }
 
 void GncPreSplit::set (GncTransPropType prop_type, const std::string& value)
 {
-    Account *acct = nullptr;
+    try
-    switch (prop_type)
     {
-        case GncTransPropType::ACTION:
+        // Drop any existing error for the prop_type we're about to set
-            m_action = boost::none;
+        m_errors.erase(prop_type);
-            if (!value.empty())
-                m_action = value;
-            break;
 
-        case GncTransPropType::TACTION:
+        Account *acct = nullptr;
-            m_taction = boost::none;
+        switch (prop_type)
-            if (!value.empty())
+        {
-                m_taction = value;
+            case GncTransPropType::ACTION:
-            break;
+                m_action = boost::none;
+                if (!value.empty())
+                    m_action = value;
+                break;
 
-        case GncTransPropType::ACCOUNT:
+            case GncTransPropType::TACTION:
-            m_account = boost::none;
+                m_taction = boost::none;
-            acct = gnc_csv_account_map_search (value.c_str());
+                if (!value.empty())
-            if (acct)
+                    m_taction = value;
-                m_account = acct;
+                break;
-            else
-                throw std::invalid_argument ("String can't be mapped back to an account.");
-            break;
 
-        case GncTransPropType::TACCOUNT:
+            case GncTransPropType::ACCOUNT:
-            m_taccount = boost::none;
+                m_account = boost::none;
-            acct = gnc_csv_account_map_search (value.c_str());
+                if (value.empty())
-            if (acct)
+                    throw std::invalid_argument (_("Account value can't be empty."));
-                m_taccount = acct;
+                acct = gnc_csv_account_map_search (value.c_str());
-            else
+                if (acct)
-                throw std::invalid_argument ("String can't be mapped back to an account.");
+                    m_account = acct;
-            break;
+                else
+                    throw std::invalid_argument (_(bad_acct));
+                break;
 
-        case GncTransPropType::MEMO:
+            case GncTransPropType::TACCOUNT:
-            m_memo = boost::none;
+                m_taccount = boost::none;
-            if (!value.empty())
+                if (value.empty())
-                m_memo = value;
+                    throw std::invalid_argument (_("Transfer account value can't be empty."));
-            break;
 
-        case GncTransPropType::TMEMO:
+                acct = gnc_csv_account_map_search (value.c_str());
-            m_tmemo = boost::none;
+                if (acct)
-            if (!value.empty())
+                    m_taccount = acct;
-                m_tmemo = value;
+                else
-            break;
+                    throw std::invalid_argument (_(bad_tacct));
+                break;
 
-        case GncTransPropType::DEPOSIT:
+            case GncTransPropType::MEMO:
-            m_deposit = boost::none;
+                m_memo = boost::none;
-            m_deposit = parse_amount (value, m_currency_format); // Will throw if parsing fails
+                if (!value.empty())
-            break;
+                    m_memo = value;
-        case GncTransPropType::WITHDRAWAL:
+                break;
-            m_withdrawal = boost::none;
-            m_withdrawal = parse_amount (value, m_currency_format); // Will throw if parsing fails
-            break;
 
-        case GncTransPropType::PRICE:
+            case GncTransPropType::TMEMO:
-            m_price = boost::none;
+                m_tmemo = boost::none;
-            m_price = parse_amount (value, m_currency_format); // Will throw if parsing fails
+                if (!value.empty())
-            break;
+                    m_tmemo = value;
+                break;
 
-        case GncTransPropType::REC_STATE:
+            case GncTransPropType::DEPOSIT:
-            m_rec_state = boost::none;
+                m_deposit = boost::none;
-            m_rec_state = parse_reconciled (value); // Throws if parsing fails
+                m_deposit = parse_amount (value, m_currency_format); // Will throw if parsing fails
-            break;
+                break;
+            case GncTransPropType::WITHDRAWAL:
+                m_withdrawal = boost::none;
+                m_withdrawal = parse_amount (value, m_currency_format); // Will throw if parsing fails
+                break;
 
-        case GncTransPropType::TREC_STATE:
+            case GncTransPropType::PRICE:
-            m_trec_state = boost::none;
+                m_price = boost::none;
-            m_trec_state = parse_reconciled (value); // Throws if parsing fails
+                m_price = parse_amount (value, m_currency_format); // Will throw if parsing fails
-            break;
+                break;
 
-        case GncTransPropType::REC_DATE:
+            case GncTransPropType::REC_STATE:
-            m_rec_date = boost::none;
+                m_rec_state = boost::none;
-            if (!value.empty())
+                m_rec_state = parse_reconciled (value); // Throws if parsing fails
-                m_rec_date = parse_date (value, m_date_format); // Throws if parsing fails
+                break;
-            break;
 
-        case GncTransPropType::TREC_DATE:
+            case GncTransPropType::TREC_STATE:
-            m_trec_date = boost::none;
+                m_trec_state = boost::none;
-            if (!value.empty())
+                m_trec_state = parse_reconciled (value); // Throws if parsing fails
-                m_trec_date = parse_date (value, m_date_format); // Throws if parsing fails
+                break;
-            break;
 
-        default:
+            case GncTransPropType::REC_DATE:
-            /* Issue a warning for all other prop_types. */
+                m_rec_date = boost::none;
-            PWARN ("%d is an invalid property for a split", static_cast<int>(prop_type));
+                if (!value.empty())
-            break;
+                    m_rec_date = parse_date (value, m_date_format); // Throws if parsing fails
+                break;
+
+            case GncTransPropType::TREC_DATE:
+                m_trec_date = boost::none;
+                if (!value.empty())
+                    m_trec_date = parse_date (value, m_date_format); // Throws if parsing fails
+                break;
+
+            default:
+                /* Issue a warning for all other prop_types. */
+                PWARN ("%d is an invalid property for a split", static_cast<int>(prop_type));
+                break;
+        }
+    }
+    catch (const std::invalid_argument& e)
+    {
+        auto err_str = std::string(_(gnc_csv_col_type_strs[prop_type])) +
+                       std::string(_(" could not be understood.\n")) +
+                       e.what();
+        m_errors.emplace(prop_type, err_str);
+        throw std::invalid_argument (err_str);
+    }
+    catch (const std::out_of_range& e)
+    {
+        auto err_str = std::string(_(gnc_csv_col_type_strs[prop_type])) +
+                       std::string(_(" could not be understood.\n")) +
+                       e.what();
+        m_errors.emplace(prop_type, err_str);
+        throw std::invalid_argument (err_str);
     }
-
 }
 
 void GncPreSplit::reset (GncTransPropType prop_type)
@@ -521,7 +597,8 @@ void GncPreSplit::reset (GncTransPropType prop_type)
     catch (...)
     {
         // Set with an empty string will effectively clear the property
-        // but also throw in many cases. For a reset this is fine, so catch it here.
+        // but can also set an error for the property. Clear that error here.
+        m_errors.erase(prop_type);
     }
 }
 
@@ -672,3 +749,8 @@ void GncPreSplit::create_split (Transaction* trans)
 
     created = true;
 }
+
+std::string GncPreSplit::errors (bool check_accts_mapped)
+{
+    return gen_err_str (m_errors, check_accts_mapped);
+}

src/import-export/csv-imp/gnc-trans-props.hpp

@@ -112,6 +112,7 @@ public:
     GncPreTrans(int date_format) : m_date_format{date_format} {};
 
     void set (GncTransPropType prop_type, const std::string& value);
+    void set_date_format (int date_format) { m_date_format = date_format ;}
     void reset (GncTransPropType prop_type);
     std::string verify_essentials (void);
     Transaction *create_trans (QofBook* book, gnc_commodity* currency);
@@ -132,6 +133,7 @@ public:
      */
     bool is_part_of (std::shared_ptr<GncPreTrans> parent);
     boost::optional<std::string> get_void_reason() { return m_void_reason; }
+    std::string errors();
 
 private:
     int m_date_format;
@@ -143,6 +145,8 @@ private:
     boost::optional<gnc_commodity*> m_commodity;
     boost::optional<std::string> m_void_reason;
     bool created = false;
+
+    std::map<GncTransPropType, std::string> m_errors;
 };
 
 struct GncPreSplit
@@ -152,11 +156,14 @@ public:
         m_currency_format{currency_format}{};
     void set (GncTransPropType prop_type, const std::string& value);
     void reset (GncTransPropType prop_type);
+    void set_date_format (int date_format) { m_date_format = date_format ;}
+    void set_currency_format (int currency_format) { m_currency_format = currency_format; }
     std::string verify_essentials (void);
     void create_split(Transaction* trans);
 
     Account* get_account () { if (m_account) return *m_account; else return nullptr; }
     void set_account (Account* acct) { if (acct) m_account = acct; else m_account = boost::none; }
+    std::string errors(bool check_accts_mapped);
 
 private:
     int m_date_format;
@@ -175,6 +182,8 @@ private:
     boost::optional<char> m_trec_state;
     boost::optional<time64> m_trec_date;
     bool created = false;
+
+    std::map<GncTransPropType, std::string> m_errors;
 };
 
 #endif

src/import-export/csv-imp/gnc-tx-import.cpp

@@ -67,6 +67,7 @@ GncTxImport::GncTxImport(GncImpFileFormat format)
      * gnc_csv_parse_data_free is called before all of the data is
      * initialized, only the data that needs to be freed is freed. */
     m_skip_errors = false;
+    m_req_mapped_accts = true;
     file_format(m_settings.m_file_format = format);
 }
 
@@ -115,18 +116,30 @@ GncImpFileFormat GncTxImport::file_format()
 /** Toggles the multi-split state of the importer and will subsequently
  *  sanitize the column_types list. All types that don't make sense
  *  in the new state are reset to type GncTransPropType::NONE.
+ *  Additionally the interpretation of the columns with transaction
+ *  properties changes when changing multi-split mode. So this function
+ *  will force a reparsing of the transaction properties (if there are
+ *  any) by resetting the first column with a transaction property
+ *  it encounters.
  * @param multi_split_val Boolean value with desired state (multi-split
  * vs two-split).
  */
 void GncTxImport::multi_split (bool multi_split)
 {
+    auto trans_prop_seen = false;
     m_settings.m_multi_split = multi_split;
-    for (auto col_it = m_settings.m_column_types.begin(); col_it != m_settings.m_column_types.end();
+    for (uint i = 0; i < m_settings.m_column_types.size(); i++)
-            col_it++)
     {
-        auto san_prop = sanitize_trans_prop (*col_it, m_settings.m_multi_split);
+        auto old_prop = m_settings.m_column_types[i];
-        if (san_prop != *col_it)
+        auto is_trans_prop = ((old_prop > GncTransPropType::NONE)
-            *col_it = san_prop;
+                && (old_prop <= GncTransPropType::TRANS_PROPS));
+        auto san_prop = sanitize_trans_prop (old_prop, m_settings.m_multi_split);
+        if (san_prop != old_prop)
+            set_column_type (i, san_prop);
+        else if (is_trans_prop && !trans_prop_seen)
+            set_column_type (i, old_prop, true);
+        trans_prop_seen |= is_trans_prop;
+
     }
     if (m_settings.m_multi_split)
         m_settings.m_base_account = nullptr;
@@ -158,18 +171,52 @@ void GncTxImport::base_account (Account* base_account)
         auto col_type = std::find (m_settings.m_column_types.begin(),
                 m_settings.m_column_types.end(), GncTransPropType::ACCOUNT);
         if (col_type != m_settings.m_column_types.end())
-            *col_type = GncTransPropType::NONE;
+            set_column_type(col_type -m_settings.m_column_types.begin(),
+                            GncTransPropType::NONE);
+
+        /* Set default account for each line's split properties */
+        for (auto line : m_parsed_lines)
+            std::get<3>(line)->set_account (m_settings.m_base_account);
+
+
     }
 }
 
 Account *GncTxImport::base_account () { return m_settings.m_base_account; }
 
+void GncTxImport::reset_formatted_column (std::vector<GncTransPropType>& col_types)
+{
+    for (auto col_type: col_types)
+    {
+        auto col = std::find (m_settings.m_column_types.begin(),
+                m_settings.m_column_types.end(), col_type);
+        if (col != m_settings.m_column_types.end())
+            set_column_type (col - m_settings.m_column_types.begin(), col_type, true);
+    }
+}
+
 void GncTxImport::currency_format (int currency_format)
-    { m_settings.m_currency_format = currency_format; }
+{
+    m_settings.m_currency_format = currency_format;
+
+    /* Reparse all currency related columns */
+    std::vector<GncTransPropType> commodities = { GncTransPropType::DEPOSIT,
+            GncTransPropType::WITHDRAWAL,
+            GncTransPropType::PRICE};
+    reset_formatted_column (commodities);
+}
 int GncTxImport::currency_format () { return m_settings.m_currency_format; }
 
 void GncTxImport::date_format (int date_format)
-    { m_settings.m_date_format = date_format; }
+{
+    m_settings.m_date_format = date_format;
+
+    /* Reparse all date related columns */
+    std::vector<GncTransPropType> dates = { GncTransPropType::DATE,
+            GncTransPropType::REC_DATE,
+            GncTransPropType::TREC_DATE};
+    reset_formatted_column (dates);
+}
 int GncTxImport::date_format () { return m_settings.m_date_format; }
 
 /** Converts raw file data using a new encoding. This function must be
@@ -182,7 +229,15 @@ void GncTxImport::encoding (const std::string& encoding)
 
     // TODO investigate if we can catch conversion errors and report them
     if (m_tokenizer)
+    {
         m_tokenizer->encoding(encoding); // May throw
+        try
+        {
+            tokenize(false);
+        }
+        catch (...)
+        { };
+    }
 
     m_settings.m_encoding = encoding;
 }
@@ -286,7 +341,7 @@ void GncTxImport::load_file (const std::string& filename)
  * function.
  * Notes: - this function must be called with guessColTypes set to true once
  *          before calling it with guessColTypes set to false.
- *        - if guessColTypes is TRUE, all the column types will be set
+ *        - if guessColTypes is true, all the column types will be set
  *          GncTransPropType::NONE right now as real guessing isn't implemented yet
  * @param guessColTypes true to guess what the types of columns are based on the cell contents
  * @exception std::range_error if tokenizing failed
@@ -302,7 +357,9 @@ void GncTxImport::tokenize (bool guessColTypes)
     for (auto tokenized_line : m_tokenizer->get_tokens())
     {
         m_parsed_lines.push_back (std::make_tuple (tokenized_line, std::string(),
-                nullptr, nullptr, false));
+                std::make_shared<GncPreTrans>(date_format()),
+                std::make_shared<GncPreSplit>(date_format(), currency_format()),
+                false));
         auto length = tokenized_line.size();
         if (length > max_cols)
             max_cols = length;
@@ -317,6 +374,15 @@ void GncTxImport::tokenize (bool guessColTypes)
 
     m_settings.m_column_types.resize(max_cols, GncTransPropType::NONE);
 
+    /* Force reinterpretation of already set columns and/or base_account */
+    for (uint i = 0; i < m_settings.m_column_types.size(); i++)
+        set_column_type (i, m_settings.m_column_types[i], true);
+    if (m_settings.m_base_account)
+    {
+        for (auto line : m_parsed_lines)
+            std::get<3>(line)->set_account (m_settings.m_base_account);
+    }
+
     if (guessColTypes)
     {
         /* Guess column_types based
@@ -346,117 +412,6 @@ std::string ErrorList::str()
     return m_error.substr(0, m_error.size() - 1);
 }
 
-void GncTxImport::verify_data(ErrorList& error_msg)
-{
-    auto have_date_errors = false;
-    auto have_amount_errors = false;
-    for (uint i = 0; i < m_parsed_lines.size(); i++)
-    {
-        auto line_err = ErrorList();
-        auto line_data = std::get<0>(m_parsed_lines[i]);
-
-        /* Attempt to parse date column values */
-        auto date_col_it = std::find(m_settings.m_column_types.begin(),
-            m_settings.m_column_types.end(), GncTransPropType::DATE);
-        if (date_col_it != m_settings.m_column_types.end())
-        try
-        {
-            auto date_col = date_col_it -m_settings.m_column_types.begin();
-            auto date_str = line_data[date_col];
-            if (!m_settings.m_multi_split || !date_str.empty())
-                parse_date (date_str, date_format());
-        }
-        catch (...)
-        {
-            if (!std::get<4>(m_parsed_lines[i])) // Skipped lines don't trigger a global error
-                have_date_errors = true;
-            line_err.add_error(_("Date could not be understood"));
-        }
-
-        /* Attempt to parse reconcile date column values */
-        date_col_it = std::find(m_settings.m_column_types.begin(),
-            m_settings.m_column_types.end(), GncTransPropType::REC_DATE);
-        if (date_col_it != m_settings.m_column_types.end())
-        try
-        {
-            auto date_col = date_col_it -m_settings.m_column_types.begin();
-            auto date_str = line_data[date_col];
-            if (!date_str.empty())
-                parse_date (date_str, date_format());
-        }
-        catch (...)
-        {
-            if (!std::get<4>(m_parsed_lines[i])) // Skipped lines don't trigger a global error
-                have_date_errors = true;
-            line_err.add_error(_("Reconcile date could not be understood"));
-        }
-
-        /* Attempt to parse transfer reconcile date column values */
-        date_col_it = std::find(m_settings.m_column_types.begin(),
-            m_settings.m_column_types.end(), GncTransPropType::TREC_DATE);
-        if (date_col_it != m_settings.m_column_types.end())
-        try
-        {
-            auto date_col = date_col_it -m_settings.m_column_types.begin();
-            auto date_str = line_data[date_col];
-            if (!date_str.empty())
-                parse_date (date_str, date_format());
-        }
-        catch (...)
-        {
-            if (!std::get<4>(m_parsed_lines[i])) // Skipped lines don't trigger a global error
-                have_date_errors = true;
-            line_err.add_error(_("Transfer reconcile date could not be understood"));
-        }
-
-        /* Attempt to parse deposit column values */
-        auto num_col_it = std::find(m_settings.m_column_types.begin(),
-            m_settings.m_column_types.end(), GncTransPropType::DEPOSIT);
-        if (num_col_it != m_settings.m_column_types.end())
-        try
-        {
-            auto num_col = num_col_it -m_settings.m_column_types.begin();
-            auto num_str = line_data[num_col];
-            if (!m_settings.m_multi_split || !num_str.empty())
-                parse_amount (num_str, currency_format());
-        }
-        catch (...)
-        {
-            if (!std::get<4>(m_parsed_lines[i])) // Skipped lines don't trigger a global error
-                have_amount_errors = true;
-            line_err.add_error(_("Deposit amount could not be understood"));
-        }
-
-        /* Attempt to parse withdrawal column values */
-        num_col_it = std::find(m_settings.m_column_types.begin(),
-            m_settings.m_column_types.end(), GncTransPropType::WITHDRAWAL);
-        if (num_col_it != m_settings.m_column_types.end())
-        try
-        {
-            auto num_col = num_col_it -m_settings.m_column_types.begin();
-            auto num_str = line_data[num_col];
-            if (!m_settings.m_multi_split || !num_str.empty())
-                parse_amount (num_str, currency_format());
-        }
-        catch (...)
-        {
-            if (!std::get<4>(m_parsed_lines[i])) // Skipped lines don't trigger a global error
-                have_amount_errors = true;
-            line_err.add_error(_("Withdrawal amount could not be understood"));
-        }
-
-        if (!line_err.empty())
-            std::get<1>(m_parsed_lines[i]) = line_err.str();
-        else
-            std::get<1>(m_parsed_lines[i]).clear();
-    }
-
-    if (have_date_errors)
-        error_msg.add_error( _("Not all dates could be parsed. Please verify your chosen date format or adjust the lines to skip."));
-    if (have_amount_errors)
-        error_msg.add_error( _("Not all amounts could be parsed. Please verify your chosen currency format or adjust the lines to skip."));
-}
-
 
 /* Test for the required minimum number of columns selected and
  * the selection is consistent.
@@ -504,8 +459,11 @@ void GncTxImport::verify_column_selections (ErrorList& error_msg)
 }
 
 
-/* Test for the required minimum number of columns selected and
+/* Check whether the chosen settings can successfully parse
- * a valid date format.
+ * the import data. This will check:
+ * - there's at least one line selected for import
+ * - the minimum number of columns is selected
+ * - the values in the selected columns can be parsed meaningfully.
  * @return An empty string if all checks passed or the reason
  *         verification failed otherwise.
  */
@@ -530,7 +488,22 @@ std::string GncTxImport::verify ()
     }
 
     verify_column_selections (error_msg);
-    verify_data (error_msg);
+
+    update_skipped_lines (boost::none, boost::none, boost::none, boost::none);
+
+    auto have_line_errors = false;
+    for (auto line : m_parsed_lines)
+    {
+        if (!std::get<4>(line) && !std::get<1>(line).empty())
+        {
+            have_line_errors = true;
+            break;
+        }
+    }
+
+    if (have_line_errors)
+        error_msg.add_error( _("Not all fields could be parsed. Please correct the issues reported for each line or adjust the lines to skip."));
+
     return error_msg.str();
 }
 
@@ -629,71 +602,16 @@ void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parse
 {
     StrVec line;
     std::string error_message;
-    auto trans_props = std::make_shared<GncPreTrans>(date_format());
+    std::shared_ptr<GncPreTrans> trans_props = nullptr;
-    auto split_props = std::make_shared<GncPreSplit>(date_format(), currency_format());
+    std::shared_ptr<GncPreSplit> split_props = nullptr;
-    std::tie(line, error_message, std::ignore, std::ignore, std::ignore) = *parsed_line;
+    bool skip_line = false;
+    std::tie(line, error_message, trans_props, split_props, skip_line) = *parsed_line;
+
+    if (skip_line)
+        return;
+
     error_message.clear();
 
-    /* Convert all tokens in this line into transaction/split properties. */
-    auto col_types_it = m_settings.m_column_types.cbegin();
-    auto line_it = line.cbegin();
-    for (col_types_it, line_it;
-            col_types_it != m_settings.m_column_types.cend() &&
-            line_it != line.cend();
-            ++col_types_it, ++line_it)
-    {
-        try
-        {
-            if (*col_types_it == GncTransPropType::NONE)
-                continue; /* We do nothing with "None"-type columns. */
-            else if  (*col_types_it <= GncTransPropType::TRANS_PROPS)
-            {
-                if (m_settings.m_multi_split && line_it->empty())
-                    continue; // In multi-split mode, transaction properties can be empty
-                trans_props->set(*col_types_it, *line_it);
-            }
-            else
-                split_props->set(*col_types_it, *line_it);
-        }
-        catch (const std::exception& e)
-        {
-            if (!error_message.empty())
-                error_message += "\n";
-            error_message += _(gnc_csv_col_type_strs[*col_types_it]);
-            error_message += _(" column could not be understood.");
-            PINFO("User warning: %s", error_message.c_str());
-        }
-    }
-    std::get<2>(*parsed_line) = trans_props;
-
-    /* For multi-split input data, we need to check whether this line is part of a transaction that
-     * has already be started by a previous line. */
-    if (m_settings.m_multi_split)
-    {
-        if (trans_props->is_part_of(m_parent))
-        {
-            /* This line is part of an already started transaction
-             * continue with that one instead to make sure the split from this line
-             * gets added to the proper transaction */
-            std::get<2>(*parsed_line) = m_parent;
-
-            /* Check if the parent line is ready for conversion. If not,
-             * this child line can't be converted either.
-             */
-            if (!m_parent->verify_essentials().empty())
-                error_message = _("First line of this transaction has errors.");
-        }
-        else
-        {
-            /* This line starts a new transaction, set it as parent for
-             * subsequent lines. */
-            m_parent = trans_props;
-        }
-    }
-
-    if (!error_message.empty())
-        throw std::invalid_argument (error_message);
-
     // Add an ACCOUNT property with the default account if no account column was set by the user
     auto line_acct = split_props->get_account();
     if (!line_acct)
@@ -710,7 +628,6 @@ void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parse
             throw std::invalid_argument(error_message);
         }
     }
-    std::get<3>(*parsed_line) = split_props;
 
     /* If column parsing was successful, convert trans properties into a draft transaction. */
     try
@@ -734,17 +651,17 @@ void GncTxImport::create_transaction (std::vector<parse_line_t>::iterator& parse
 
 
 /** Creates a list of transactions from parsed data. The parsed data
- * will first be validated. If any errors are found this function will
+ * will first be validated. If any errors are found in lines that are marked
- * throw an error unless skip_errors was set.
+ * for processing (ie not marked to skip) this function will
+ * throw an error.
  * @param skip_errors true skip over lines with errors
- * @exception throws std::invalid_argument if data validation fails and
+ * @exception throws std::invalid_argument if data validation or processing fails.
- *            skip_errors wasn't set.
  */
 void GncTxImport::create_transactions ()
 {
     /* Start with verifying the current data. */
     auto verify_result = verify();
-    if (!verify_result.empty() && !m_skip_errors)
+    if (!verify_result.empty())
         throw std::invalid_argument (verify_result);
 
     /* Drop all existing draft transactions */
@@ -761,15 +678,8 @@ void GncTxImport::create_transactions ()
         if ((std::get<4>(*parsed_lines_it)))
             continue;
 
-        try
+        /* Should not throw anymore, otherwise verify needs revision */
-        {
+        create_transaction (parsed_lines_it);
-            create_transaction (parsed_lines_it);
-        }
-        catch (const std::invalid_argument& e)
-        {
-            std::get<1>(*parsed_lines_it) = e.what();
-            continue;
-        }
     }
 }
 
@@ -782,20 +692,157 @@ GncTxImport::check_for_column_type (GncTransPropType type)
                         != m_settings.m_column_types.end());
 }
 
+/* A helper function intended to be called only from set_column_type */
+void GncTxImport::update_pre_trans_props (uint row, uint col, GncTransPropType prop_type)
+{
+    if ((prop_type == GncTransPropType::NONE) || (prop_type > GncTransPropType::TRANS_PROPS))
+        return; /* Only deal with transaction related properties. */
+
+    auto trans_props = std::make_shared<GncPreTrans> (*(std::get<2>(m_parsed_lines[row])).get());
+    auto value = std::string();
+
+    if (col < std::get<0>(m_parsed_lines[row]).size())
+        value = std::get<0>(m_parsed_lines[row]).at(col);
+
+    if (value.empty())
+        trans_props->reset (prop_type);
+    else
+    {
+        try
+        {
+            trans_props->set(prop_type, value);
+        }
+        catch (const std::exception& e)
+        {
+            /* Do nothing, just prevent the exception from escalating up
+             * However log the error if it happens on a row that's not skipped
+             */
+            if (!std::get<4>(m_parsed_lines[row]))
+                PINFO("User warning: %s", e.what());
+        }
+    }
+
+    /* Store the result */
+    std::get<2>(m_parsed_lines[row]) = trans_props;
+
+    /* For multi-split input data, we need to check whether this line is part of
+     * a transaction that has already been started by a previous line. */
+    if (m_settings.m_multi_split)
+    {
+        if (trans_props->is_part_of(m_parent))
+        {
+            /* This line is part of an already started transaction
+             * continue with that one instead to make sure the split from this line
+             * gets added to the proper transaction */
+            std::get<2>(m_parsed_lines[row]) = m_parent;
+        }
+        else
+        {
+            /* This line starts a new transaction, set it as parent for
+             * subsequent lines. */
+            m_parent = trans_props;
+        }
+    }
+}
+
+/* A helper function intended to be called only from set_column_type */
+void GncTxImport::update_pre_split_props (uint row, uint col, GncTransPropType prop_type)
+{
+    if ((prop_type > GncTransPropType::SPLIT_PROPS) || (prop_type <= GncTransPropType::TRANS_PROPS))
+        return; /* Only deal with split related properties. */
+
+    auto split_props = std::get<3>(m_parsed_lines[row]);
+    auto value = std::string();
+
+    if (col < std::get<0>(m_parsed_lines[row]).size())
+        value = std::get<0>(m_parsed_lines[row]).at(col);
+
+    if (value.empty())
+        split_props->reset (prop_type);
+    else
+    {
+        try
+        {
+            split_props->set(prop_type, value);
+        }
+        catch (const std::exception& e)
+        {
+            /* Do nothing, just prevent the exception from escalating up
+             * However log the error if it happens on a row that's not skipped
+             */
+            if (!std::get<4>(m_parsed_lines[row]))
+                PINFO("User warning: %s", e.what());
+        }
+    }
+}
+
+
 void
-GncTxImport::set_column_type (uint position, GncTransPropType type)
+GncTxImport::set_column_type (uint position, GncTransPropType type, bool force)
 {
     if (position >= m_settings.m_column_types.size())
         return;
 
+    auto old_type = m_settings.m_column_types[position];
+    if ((type == old_type) && !force)
+        return; /* Nothing to do */
+
     // Column types should be unique, so remove any previous occurrence of the new type
     std::replace(m_settings.m_column_types.begin(), m_settings.m_column_types.end(),
             type, GncTransPropType::NONE);
+
     m_settings.m_column_types.at (position) = type;
 
     // If the user has set an Account column, we can't have a base account set
     if (type == GncTransPropType::ACCOUNT)
         base_account (nullptr);
+
+    /* Update the preparsed data */
+    m_parent = nullptr;
+    for (auto parsed_lines_it = m_parsed_lines.begin();
+            parsed_lines_it != m_parsed_lines.end();
+            ++parsed_lines_it)
+    {
+        /* Reset date and currency formats for each trans/split props object
+         * to ensure column updates use the most recent one
+         */
+        std::get<2>(*parsed_lines_it)->set_date_format (m_settings.m_date_format);
+        std::get<3>(*parsed_lines_it)->set_date_format (m_settings.m_date_format);
+        std::get<3>(*parsed_lines_it)->set_currency_format (m_settings.m_currency_format);
+
+        uint row = parsed_lines_it - m_parsed_lines.begin();
+
+        /* If the column type actually changed, first reset the property
+         * represented by the old column type
+         */
+        if (old_type != type)
+        {
+            auto old_col = std::get<0>(*parsed_lines_it).size(); // Deliberately out of bounds to trigger a reset!
+            if ((old_type > GncTransPropType::NONE)
+                    && (old_type <= GncTransPropType::TRANS_PROPS))
+                update_pre_trans_props (row, old_col, old_type);
+            else if ((old_type > GncTransPropType::TRANS_PROPS)
+                    && (old_type <= GncTransPropType::SPLIT_PROPS))
+                update_pre_split_props (row, old_col, old_type);
+        }
+
+        /* Then set the property represented by the new column type */
+        if ((type > GncTransPropType::NONE)
+                && (type <= GncTransPropType::TRANS_PROPS))
+            update_pre_trans_props (row, position, type);
+        else if ((type > GncTransPropType::TRANS_PROPS)
+                && (type <= GncTransPropType::SPLIT_PROPS))
+            update_pre_split_props (row, position, type);
+
+        /* Report errors if there are any */
+        auto trans_errors = std::get<2>(*parsed_lines_it)->errors();
+        auto split_errors = std::get<3>(*parsed_lines_it)->errors(m_req_mapped_accts);
+        std::get<1>(*parsed_lines_it) =
+                trans_errors +
+                (trans_errors.empty() && split_errors.empty() ? std::string() : "\n") +
+                split_errors;
+
+    }
 }
 
 std::vector<GncTransPropType> GncTxImport::column_types ()

src/import-export/csv-imp/gnc-tx-import.hpp

@@ -123,6 +123,8 @@ public:
     bool skip_alt_lines ();
     bool skip_err_lines ();
 
+    void req_mapped_accts (bool val) {m_req_mapped_accts = val; }
+
     void separators (std::string separators);
     std::string separators ();
 
@@ -144,7 +146,7 @@ public:
      */
     void create_transactions ();
     bool check_for_column_type (GncTransPropType type);
-    void set_column_type (uint position, GncTransPropType type);
+    void set_column_type (uint position, GncTransPropType type, bool force = false);
     std::vector<GncTransPropType> column_types ();
 
     std::set<std::string> accounts ();
@@ -164,19 +166,29 @@ private:
     void create_transaction (std::vector<parse_line_t>::iterator& parsed_line);
 
     void verify_column_selections (ErrorList& error_msg);
-    void verify_data(ErrorList& error_msg);
+
+    /* Internal helper function to force reparsing of columns subject to format changes */
+    void reset_formatted_column (std::vector<GncTransPropType>& col_types);
 
     /* Internal helper function that does the actual conversion from property lists
      * to real (possibly unbalanced) transaction with splits.
      */
     std::shared_ptr<DraftTransaction> trans_properties_to_trans (std::vector<parse_line_t>::iterator& parsed_line);
 
+    /* Two internal helper functions that should only be called from within
+     * set_column_type for consistency (otherwise error messages may not be (re)set)
+     */
+    void update_pre_trans_props (uint row, uint col, GncTransPropType prop_type);
+    void update_pre_split_props (uint row, uint col, GncTransPropType prop_type);
+
     struct CsvTranSettings;
     CsvTransSettings m_settings;
     bool m_skip_errors;
+    bool m_req_mapped_accts;
 
     /* The parameters below are only used while creating
-     * transactions. They keep state information during the conversion.
+     * transactions. They keep state information while processing multi-split
+     * transactions.
      */
     std::shared_ptr<GncPreTrans> m_parent = nullptr;
     std::shared_ptr<DraftTransaction> m_current_draft = nullptr;