Bug 330737

Summary: csv export mess separation comma with digit comma
Product: [Applications] kmymoney Reporter: luisfe <luisfe>
Component: importerAssignee: KMyMoney Devel Mailing List <kmymoney-devel>
Status: RESOLVED FIXED    
Severity: normal CC: agander93, debrabant.philippe, onet.cristian
Priority: NOR    
Version: git (master)   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 4.7.2
Sentry Crash Report:
Attachments: Patch to allow selection of separator.

Description luisfe 2014-02-04 09:11:35 UTC
I f I eexport an account to csv format, the separator delimiter is a comma. Unfortunately decimals in quantities are also separated by a comma. It is a mess to open it for instance with calligrasheets or openoffice calc, because cents appear of their own column.

Reproducible: Always

Steps to Reproduce:
1. Choose an account whose quantities have decimals
2. Export the account to csv
3. Try to open the csv file with a spreadsheet program.
Comment 1 allan 2014-02-04 11:15:15 UTC
'Mess', I think, describes it pretty well, I'm afraid.  Obviously, I somehow failed to take that into account, which is a pretty basic mistake, for which I ahve to apologise.
I'll take a look.
Comment 2 Cristian Oneț 2014-10-29 14:14:29 UTC
Allan how is the fix for this coming along?
Comment 3 allan 2014-10-29 14:26:39 UTC
(In reply to Cristian Oneț from comment #2)
> Allan how is the fix for this coming along?

It's pretty close, but I'm afraid I side-tracked myself.  I was re-importing the exported file, and realised that the importer had no capability to import the transaction status, as this doesn't appear in conventional import files.  So, I decided to add it to the importer.  That brought me back to the cluttered banking and investment wizard pages, and I set about reworking them.

This was stupid, so I'll get back to the original issue and finish it off.
Comment 4 Cristian Oneț 2014-10-29 14:48:10 UTC
Nice, just take one step at a time :).
Comment 5 allan 2014-10-30 22:48:41 UTC
I've realised during the testing, that the exporter doesn't differentiate between accounts and categories when faced with a transfer.  I decided to use the QIF approach, and enclosed the account name in [ ], which would be stripped during import. 

 I then realised that the importer also doesn't differentiate.  I was at the point of modifying the importer, to remove the [ ], when it occured to me that that wasn't such a good idea, as a user importing a completely different style of CSV file probably wouldn't differentiate in the same way.  In fact, assuming that most non-investment CSV files would come from a bank, which would have no knowledge of the KMM account structure, transfers probaly would be few and far between.

So, a possible alternative which would put the user in control, would be to allow him/her to choose different columns for categories and accounts when importing.  The consequence of that is that the exporter wouldn't need to differentiate either, leaving it for the user, when importing to allocate columns as required.

Any thoughts?
Comment 6 Jack 2014-10-30 22:57:51 UTC
What's the purpose of exporting?  That should tell you what information is really necessary.  I know you don't know the variety of ways it will be used, but I think it's ok to design against a limited set of use cases if it can't be made completely generalizable.

For me, CSV is not the optimal way to move info between different instances of KMM, so account structure doesn't seem important.  I would consider using csv export for transfer not to another accounting app, but to graphics or modelling program, in which case I don't think I'd be likely to confuse accounts/payees/categories/.....  However, if there is a column naming one of them, how about an optional following column with a key letter for the type of account being named?
Comment 7 allan 2014-10-31 12:27:14 UTC
On 30/10/14 22:57, Jack wrote:
> https://bugs.kde.org/show_bug.cgi?id=330737
>
> Jack <ostroffjh@users.sourceforge.net> changed:
>
>             What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                   CC|                            |ostroffjh@users.sourceforge
>                     |                            |.net
>
> --- Comment #6 from Jack <ostroffjh@users.sourceforge.net> ---
> What's the purpose of exporting?  That should tell you what information is
> really necessary.  I know you don't know the variety of ways it will be used,
> but I think it's ok to design against a limited set of use cases if it can't be
> made completely generalizable.
>
> For me, CSV is not the optimal way to move info between different instances of
> KMM, so account structure doesn't seem important.  I would consider using csv
> export for transfer not to another accounting app, but to graphics or modelling
> program, in which case I don't think I'd be likely to confuse
> accounts/payees/categories/.....

But how does the actual user want to use it?  We don't know, and he 
might have another KMM (or, Heaven forbid, GnuCash)file into which he 
wants to copy an existing account, or just one transaction.  So, I don't 
think it right to convert an account name (transfer) into a category name.

> However, if there is a column naming one of
> them, how about an optional following column with a key letter for the type of
> account being named?

I'm not sure what that would gain.  If there is to be another column, 
why not make it more obvious and name it Transfer account, or whatever, 
rather than Cat/Accnt?

Allan
Comment 8 allan 2014-11-04 12:33:00 UTC
(In reply to Cristian Oneț from comment #4)
> Nice, just take one step at a time :).

I'm attaching the patch for this bug, and just this bug, which allows selection of field separator.

The use of a colon as field separator would clash with the category:subcategory colon, so temporarily substitute # as field separator in such a situation. This will require a corresponding change in the csvdialog() class, which will follow.

If there are no objectons to the patch, I'll commit in a weerk or so.
Comment 9 allan 2014-11-04 12:35:36 UTC
Created attachment 89437 [details]
Patch to allow selection of separator.
Comment 10 Jack 2014-11-04 14:14:25 UTC
When you say "temporarily substitute" I'm not sure if you mean just for this export, or some shorter time.  If the field separator exists within a field, I would either quote the entire field or escape that character.  As a user, I would not be happy if the application changed the separator I requested.
Comment 11 allan 2014-11-04 14:46:57 UTC
(In reply to Jack from comment #10)
> When you say "temporarily substitute" I'm not sure if you mean just for this
> export, or some shorter time.  If the field separator exists within a field,
> I would either quote the entire field or escape that character.  As a user,
> I would not be happy if the application changed the separator I requested.

An interesting point.  Firstly, would you as an informed user select a field separator that
is bound to clash with a category splitter?  Whilst you may not be intending to import the
exported file back into KMM, never-the-less, those colons are going to split a complex category
into separate fields in the export file.  Is that desirable?  I wouldn't have thought so.

The use of the expression "temporarily substitute" is probably misleading.  With my blinkers on, and while testing by importing into KMM the exported file, the substituted colons would be reinserted during import.  To that extent, the '# ' separators are temporary.  However, they still remain in the exported file, so in that respect, they are not temporary.

I have followed the csv importer, in giving a choice of four separators ( ',' -  '; ' - ':' - tab).  Perhaps the choice of the potentially disruptive ':' should not be given?  Or, more simply, for me, instead give a dire warning of the risk in their use?
Comment 12 allan 2014-11-05 00:36:40 UTC
On 04/11/14 17:56, Jack wrote:> I'll reply to the bug later - I can't adequately get to BKO with my 
> tablet or with the laptop I'm using right now.....
> i think our main difference is that you are thinking of this 
> specifically as a way to export some data from KMM, and I'm thinking of 
> it in the context of a general delimited file exporter.  In general 
> terms of delimited files, I have no problem with using a delimiter that 
> might occur within one of the fields, because I expect that either the 
> field will be quoted (either ' or " - hopefully one of those is not 
> within the quoted field) or the delimiter within a field will be 
> escaped, usually with a backslash.

Of course, in this specific instance, the Category value is not quoted and the colon is not escaped.


> It's certainly a valid approach 
> for this exporter to restrict options to minimize the chance of clashes, 
> but I'd also for example allow the possibility of a # within a payee 
> name (especially if it originally got created during an import).
> I think any approach here can be valid - just be constent and document 
> (for the user) what the restrictions are - principle of least surprise.
> Jack

I think the cleanest, safest, approach is to remove the potential source of problems and confusion, the colon as field delimiter.

There will therefore be no filtering of # characters in any alpha field.

I have now removed the colon option, and also the code dealing with its interception.  This has now been tested and all looks good to me.    I'll wait a while before committing, as I said earlier.
Comment 13 allan 2014-12-13 00:15:02 UTC
Git commit 00ec9fa1d3860f7ce4f9d3ba46d8a7a6ba1ee329 by Allan Anderson.
Committed on 13/12/2014 at 00:09.
Pushed by allananderson into branch 'master'.
Fix problem with hard-coded field separators, by adding selection to the UI. The use of a colon as field separator would clash with the category:subcategory use, so is excluded.

Export of Reinv and Div transactions is improved.

M  +13   -1    kmymoney/plugins/csvexport/csvexportdlg.cpp
M  +15   -0    kmymoney/plugins/csvexport/csvexportdlg.h
M  +36   -2    kmymoney/plugins/csvexport/csvexportdlg.ui
M  +2    -1    kmymoney/plugins/csvexport/csvexporterplugin.cpp
M  +71   -63   kmymoney/plugins/csvexport/csvwriter.cpp
M  +7    -4    kmymoney/plugins/csvexport/csvwriter.h

http://commits.kde.org/kmymoney/00ec9fa1d3860f7ce4f9d3ba46d8a7a6ba1ee329
Comment 14 allan 2014-12-13 17:20:46 UTC
Git commit 9f8f78edeba974b01c762bb9c5e75bfd714a85e5 by Allan Anderson.
Committed on 13/12/2014 at 17:07.
Pushed by allananderson into branch 'master'.
Fix problem in commit 00ec9fa1d3860f7ce4f9d3ba46d8a7a6ba1ee329.
Selecting tab as field separator resulted in colon being used, even
though the UI did not present the colon as a possible selection.

Please enter the commit message for your changes. Lines starting

M  +1    -1    kmymoney/plugins/csvexport/csvexportdlg.cpp

http://commits.kde.org/kmymoney/9f8f78edeba974b01c762bb9c5e75bfd714a85e5
Comment 15 Cristian Oneț 2014-12-23 12:08:31 UTC
Git commit e177bc3875084d06b0f141150ddddbc8726eb82c by Cristian Oneț, on behalf of Allan Anderson.
Committed on 13/12/2014 at 00:09.
Pushed by conet into branch '4.7'.
Fix problem with hard-coded field separators, by adding selection to the UI. The use of a colon as field separator would clash with the category:subcategory use, so is excluded.

Export of Reinv and Div transactions is improved.

(cherry picked from commit 00ec9fa1d3860f7ce4f9d3ba46d8a7a6ba1ee329)

M  +13   -1    kmymoney/plugins/csvexport/csvexportdlg.cpp
M  +15   -0    kmymoney/plugins/csvexport/csvexportdlg.h
M  +36   -2    kmymoney/plugins/csvexport/csvexportdlg.ui
M  +2    -1    kmymoney/plugins/csvexport/csvexporterplugin.cpp
M  +71   -63   kmymoney/plugins/csvexport/csvwriter.cpp
M  +7    -4    kmymoney/plugins/csvexport/csvwriter.h

http://commits.kde.org/kmymoney/e177bc3875084d06b0f141150ddddbc8726eb82c
Comment 16 Cristian Oneț 2014-12-23 12:08:31 UTC
Git commit 5f85c0074bfd9dd4cbd3743ee492d055f602c570 by Cristian Oneț, on behalf of Allan Anderson.
Committed on 13/12/2014 at 17:07.
Pushed by conet into branch '4.7'.
Fix problem in commit 00ec9fa1d3860f7ce4f9d3ba46d8a7a6ba1ee329.
Selecting tab as field separator resulted in colon being used, even
though the UI did not present the colon as a possible selection.

Please enter the commit message for your changes. Lines starting

(cherry picked from commit 9f8f78edeba974b01c762bb9c5e75bfd714a85e5)

M  +1    -1    kmymoney/plugins/csvexport/csvexportdlg.cpp

http://commits.kde.org/kmymoney/5f85c0074bfd9dd4cbd3743ee492d055f602c570
Comment 17 allan 2015-03-29 13:30:32 UTC
Git commit b76869b7b22645edbce617541dbe161e6b557d99 by Allan Anderson.
Committed on 13/12/2014 at 00:09.
Pushed by allananderson into branch '4.7'.
Fix problem with hard-coded field separators, by adding selection to the UI. The use of a colon as field separator would clash with the category:subcategory use, so is excluded.

Export of Reinv and Div transactions is improved.

(cherry picked from commit 00ec9fa1d3860f7ce4f9d3ba46d8a7a6ba1ee329)

Conflicts:
	kmymoney/plugins/csvexport/csvexportdlg.cpp

M  +1    -1    kmymoney/plugins/csvexport/csvexportdlg.cpp

http://commits.kde.org/kmymoney/b76869b7b22645edbce617541dbe161e6b557d99
Comment 18 allan 2015-03-29 13:42:27 UTC
Git commit 7dd5fde7ace481635e65d8e7d7616f93902f0a31 by Allan Anderson.
Committed on 13/12/2014 at 17:07.
Pushed by allananderson into branch '4.7'.
Fix problem in commit 00ec9fa1d3860f7ce4f9d3ba46d8a7a6ba1ee329.
Selecting tab as field separator resulted in colon being used, even
though the UI did not present the colon as a possible selection.

Please enter the commit message for your changes. Lines starting

(cherry picked from commit 9f8f78edeba974b01c762bb9c5e75bfd714a85e5)
(cherry picked from commit 5f85c0074bfd9dd4cbd3743ee492d055f602c570)

M  +1    -1    kmymoney/plugins/csvexport/csvexportdlg.cpp

http://commits.kde.org/kmymoney/7dd5fde7ace481635e65d8e7d7616f93902f0a31
Comment 19 Philippe Debrabant 2015-11-22 10:17:41 UTC
Hello Allan,
I use the Linux 4.7.1 version and the new property is not displayed.
For me "Pushed by allananderson into branch '4.7'." seems to say available from the 4.7 version.
Am I wrong ?
Comment 20 allan 2015-11-23 13:58:43 UTC
(In reply to Philippe Debrabant from comment #19)
> Hello Allan,
> I use the Linux 4.7.1 version and the new property is not displayed.
> For me "Pushed by allananderson into branch '4.7'." seems to say available
> from the 4.7 version.
> Am I wrong ?

Hello Philippe
"Pushed by allananderson into branch '4.7'." appears in several places in this bug, and they all refer to export of CSV.
I'm not quite sure which "new property is not displayed".
Can you expand, please?
Comment 21 Philippe Debrabant 2015-11-23 19:26:28 UTC
It's about the possibility to select the field separator.
Comment 22 allan 2015-11-23 20:32:10 UTC
(In reply to Philippe Debrabant from comment #21)
> It's about the possibility to select the field separator.

OK, and, again, as this bug references CSV export, you want to export a file, selecting the separator.

I have just done that, choosing semi-colon as the separator.  The produced file was then imported into KMM, and appeared correctly formatted, and showing semi-colon as the separator.

This is on the stable 4.7.2.

Allan
Comment 23 Philippe Debrabant 2015-11-24 16:20:31 UTC
Ok, I just installed the 4.7.2 version and I can change the field separator now.
Thank you for your answers.
Philippe