Re: [GNC-dev] gnucash maint: Bug 796484 - csv import: iostream error

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [GNC-dev] gnucash maint: Bug 796484 - csv import: iostream error

Geert Janssens-4
I have mixed feelings about this fix.

It's concise and does the job but on the other hand it reintroduces a
dependency on glib in otherwise clean c++ code... Getting rid of glib is one
of our long term goals. Is there a c++ alternative or would this require using
the native Windows api in some way ?

Geert

Op dinsdag 12 juni 2018 20:48:51 CEST schreef John Ralls:

> Updated via  https://github.com/Gnucash/gnucash/commit/91795052 (commit)
> from  https://github.com/Gnucash/gnucash/commit/9e6760f7 (commit)
>
>
>
> commit 9179505208768fb718b85c50b29b06bf732547f3
> Author: John Ralls <[hidden email]>
> Date:   Tue Jun 12 11:43:25 2018 -0700
>
>     Bug 796484 - csv import: iostream error
>
>     Unfortunately it turns out that we can't use filestreams because
>     they can't take path arguments containing Unicode on Windows.
>     Replace the filestream code with g_file_get_contents(),  which
>     takes care of all of that Windows compatibility noise for us.
>
> diff --git a/gnucash/import-export/csv-imp/gnc-tokenizer.cpp
> b/gnucash/import-export/csv-imp/gnc-tokenizer.cpp index 7b14a2d..f8917f7
> 100644
> --- a/gnucash/import-export/csv-imp/gnc-tokenizer.cpp
> +++ b/gnucash/import-export/csv-imp/gnc-tokenizer.cpp
> @@ -15,6 +15,8 @@
>
>  extern "C" {
>  #include <go-glib-extras.h>
> +#include <glib.h>
> +#include <glib/gstdio.h>
>  }
>
>  std::unique_ptr<GncTokenizer> gnc_tokenizer_factory(GncImpFileFormat fmt)
> @@ -43,18 +45,16 @@ GncTokenizer::load_file(const std::string& path)
>          return;
>
>      m_imp_file_str = path;
> +    char *raw_contents;
> +    size_t raw_length;
> +    GError *error;
>
> -    std::ifstream in;
> -    in.exceptions ( std::ifstream::failbit | std::ifstream::badbit );
> -    in.open (m_imp_file_str.c_str(), std::ios::in | std::ios::binary);
> -
> -    m_raw_contents.clear();
> -    in.seekg(0, std::ios::end);
> -    m_raw_contents.resize(in.tellg());
> -    in.seekg(0, std::ios::beg);
> -    in.read(&m_raw_contents[0], m_raw_contents.size());
> -    in.close();
> +    if (!g_file_get_contents(path.c_str(), &raw_contents, &raw_length,
> &error)) +      throw std::ifstream::failure(error->message);
>
> +    m_raw_contents = raw_contents;
> +    g_free(raw_contents);
> +
>      // Guess encoding, user can override if needed later on.
>      const char *guessed_enc = NULL;
>      guessed_enc = go_guess_encoding (m_raw_contents.c_str(),
>
>
>
> Summary of changes:
>  gnucash/import-export/csv-imp/gnc-tokenizer.cpp | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> _______________________________________________
> gnucash-changes mailing list
> [hidden email]
> https://lists.gnucash.org/mailman/listinfo/gnucash-changes




_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: [GNC-dev] gnucash maint: Bug 796484 - csv import: iostream error

John Ralls-2
Geert,

I have mixed feelings about it too. Unfortunately there’s no way to use file streams with wchar_t paths with gcc. Visual C++’s c++ standard library naturally does support it, but it will take a lot of work to get GnuCash building with VC++.

We could make a gnc_fopen copying the code for g_fopen into core utils somewhere and implement a c++-sh gnc_file_get_contents around it to remove that dependency on glib.

Regards,
John Ralls

> On Jun 13, 2018, at 12:58 AM, Geert Janssens <[hidden email]> wrote:
>
> I have mixed feelings about this fix.
>
> It's concise and does the job but on the other hand it reintroduces a
> dependency on glib in otherwise clean c++ code... Getting rid of glib is one
> of our long term goals. Is there a c++ alternative or would this require using
> the native Windows api in some way ?
>
> Geert
>
> Op dinsdag 12 juni 2018 20:48:51 CEST schreef John Ralls:
>> Updated via  https://github.com/Gnucash/gnucash/commit/91795052 (commit)
>> from  https://github.com/Gnucash/gnucash/commit/9e6760f7 (commit)
>>
>>
>>
>> commit 9179505208768fb718b85c50b29b06bf732547f3
>> Author: John Ralls <[hidden email]>
>> Date:   Tue Jun 12 11:43:25 2018 -0700
>>
>>    Bug 796484 - csv import: iostream error
>>
>>    Unfortunately it turns out that we can't use filestreams because
>>    they can't take path arguments containing Unicode on Windows.
>>    Replace the filestream code with g_file_get_contents(),  which
>>    takes care of all of that Windows compatibility noise for us.
>>
>> diff --git a/gnucash/import-export/csv-imp/gnc-tokenizer.cpp
>> b/gnucash/import-export/csv-imp/gnc-tokenizer.cpp index 7b14a2d..f8917f7
>> 100644
>> --- a/gnucash/import-export/csv-imp/gnc-tokenizer.cpp
>> +++ b/gnucash/import-export/csv-imp/gnc-tokenizer.cpp
>> @@ -15,6 +15,8 @@
>>
>> extern "C" {
>> #include <go-glib-extras.h>
>> +#include <glib.h>
>> +#include <glib/gstdio.h>
>> }
>>
>> std::unique_ptr<GncTokenizer> gnc_tokenizer_factory(GncImpFileFormat fmt)
>> @@ -43,18 +45,16 @@ GncTokenizer::load_file(const std::string& path)
>>         return;
>>
>>     m_imp_file_str = path;
>> +    char *raw_contents;
>> +    size_t raw_length;
>> +    GError *error;
>>
>> -    std::ifstream in;
>> -    in.exceptions ( std::ifstream::failbit | std::ifstream::badbit );
>> -    in.open (m_imp_file_str.c_str(), std::ios::in | std::ios::binary);
>> -
>> -    m_raw_contents.clear();
>> -    in.seekg(0, std::ios::end);
>> -    m_raw_contents.resize(in.tellg());
>> -    in.seekg(0, std::ios::beg);
>> -    in.read(&m_raw_contents[0], m_raw_contents.size());
>> -    in.close();
>> +    if (!g_file_get_contents(path.c_str(), &raw_contents, &raw_length,
>> &error)) +      throw std::ifstream::failure(error->message);
>>
>> +    m_raw_contents = raw_contents;
>> +    g_free(raw_contents);
>> +
>>     // Guess encoding, user can override if needed later on.
>>     const char *guessed_enc = NULL;
>>     guessed_enc = go_guess_encoding (m_raw_contents.c_str(),
>>
>>
>>
>> Summary of changes:
>> gnucash/import-export/csv-imp/gnc-tokenizer.cpp | 20 ++++++++++----------
>> 1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> _______________________________________________
>> gnucash-changes mailing list
>> [hidden email]
>> https://lists.gnucash.org/mailman/listinfo/gnucash-changes
>
>
>
>
> _______________________________________________
> gnucash-devel mailing list
> [hidden email]
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel

_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel