After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 777230 - Added german date format to CSV transactions importer
Added german date format to CSV transactions importer
Status: RESOLVED OBSOLETE
Product: GnuCash
Classification: Other
Component: Import - CSV
git-master
Other All
: Normal normal
: ---
Assigned To: gnucash-import-maint
gnucash-import-maint
Depends on:
Blocks:
 
 
Reported: 2017-01-13 17:53 UTC by Eike
Modified: 2018-06-29 23:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (707.55 KB, application/mbox)
2017-01-13 17:53 UTC, Eike
Details

Description Eike 2017-01-13 17:53:24 UTC
Created attachment 343447 [details]
Patch

The attached patch adds the standard german date format (day.month.year) to the CSV importer for transactions. Additionally, the german translation has been specified in the updated de.po file.
Comment 1 Geert Janssens 2017-01-21 12:53:27 UTC
Comment on attachment 343447 [details]
Patch

Thank you for your patch.

From my understanding however it will not do what you hope to achieve. Apart from adding an entry in the list of date formats, you should also update the code that does the actual parsing to take this new format into account.

What's more, the new date format is redundant. The one you would want is "d-m-y". While I agree this is not clear at first sight, the date formats only define the ordering of the date parts. They don't specify a separator. In fact the code doesn't care what is used as separator. So the "d-m-y" date format will happily parse all of
31/12/2016
31.12.2016
31 12 2016
31-12-2016
And even
31-12/2016
although one could argue this is a bit too flexible.

So although I do appreciate your effort to create a patch this part will be discarded.

As for the translations, it's not clear to me what you updated exactly. There are lots of changes in the file due to line numbers shifting position. Is this a general update of the German translation or was it meant as an update that goes exactly with the added date format ?
Comment 2 Geert Janssens 2017-01-21 13:02:26 UTC
Oh, and I should add 2 more things:

1. Strictly speaking the master branch isn't open for translation  work yet. All translation focus is currently on the maint branch. This is to avoid duplicate work as much as possible.
I'm not sure what to do with your translations on the master branch. If I apply it I don't know if this will cause merge conflicts when the translations from maint are merged in.

2. Secondly more specifically to your proposed improvement for the csv importer, I'd like to inform you I have been working on a major rewrite of that part of the code in the course of the last couple of months. This work hasn't been merged in yet due to a couple of outstanding issues. But the code you are changing is completely gone. If you like to check it out, feel free to look at the "cpp" branch on my personal github gnucash repository
https://github.com/gjanssens/gnucash
Comment 3 Frank H. Ellenberger 2017-01-21 14:24:29 UTC
Comment on attachment 343447 [details]
Patch

We are always happy, if we get some support in translation and documentation.
But I have the following difficulties:
Bugzilla seems not to like the mbox format to display the patch. I had to download it.

Which encoding did you use? I get no correct display of the umlauts. It should be UTF-8

If you can adjust your changes to apply on the maint branch I would be interested.

Please obey
http://wiki.gnucash.org/wiki/Translation#Updating_an_existing_.po_file
beneath the rest of that page.

E.g. there is no need to translate paragraphs starting wihh "~#". They are no longer in active use.
Comment 4 Frank H. Ellenberger 2017-01-21 14:28:11 UTC
Please tell us your decission and change the status to "New" again.
Comment 5 Eike 2017-01-21 16:43:57 UTC
Hello,

thank you for your comments. I was not aware that "d-m-y" is already able to parse dates such as "24.12.2016" but I know now.

Regarding my patch: Of course I made sure that the existing code used to parse the date formats would be able to parse "d.m.y" as well before I created the Bugzilla issue.

For the translation, I followed the wiki. I was unsure how to handle the case where just one single entry had to be created (the "d.m.y" translation). And yes, merging all missing entries just to get that single entry seemed to be a bit of an overkill. :-)

Is there a process how to provide translations just for a chosen subset of new features? Such a process might be helpful.

Conclusion: Sorry for generating unnecessary work. Please reject this issue (I do not have the required access rights to set the status to "New").

Btw. I'm happy to hear that the cpp reimplementation is ongoing. I would be interested in contributing to that. In fact, some thoughts I have about useful new Gnucash features have been the motivation for looking into the code and thinking about the csv importer...
Comment 6 Geert Janssens 2017-01-22 19:11:13 UTC
(In reply to Eike from comment #5)
> Hello,
> 
> thank you for your comments. I was not aware that "d-m-y" is already able to
> parse dates such as "24.12.2016" but I know now.
> 
> Regarding my patch: Of course I made sure that the existing code used to
> parse the date formats would be able to parse "d.m.y" as well before I
> created the Bugzilla issue.
> 
Yes, you are correct. I didn't read far enough in the code. Your patch would work indeed, even if redundant.

> For the translation, I followed the wiki. I was unsure how to handle the
> case where just one single entry had to be created (the "d.m.y"
> translation). And yes, merging all missing entries just to get that single
> entry seemed to be a bit of an overkill. :-)
> 
> Is there a process how to provide translations just for a chosen subset of
> new features? Such a process might be helpful.
> 
I'm not sure. I'm not involved that much in the translation process. However I imagine you could reduce your patch to the essential parts before creating a PR and discarding all changes in the PO file not related to your specific translation. 'git add -i' (i from interactive) can probably help with that. You'd need to be careful of course not to create an invalid patch that way.

> Conclusion: Sorry for generating unnecessary work. Please reject this issue
> (I do not have the required access rights to set the status to "New").

No problem, thank you for proposing your patch anyway.
> 
> Btw. I'm happy to hear that the cpp reimplementation is ongoing. I would be
> interested in contributing to that. In fact, some thoughts I have about
> useful new Gnucash features have been the motivation for looking into the
> code and thinking about the csv importer...

You're most welcome to join in the work. I suggest you introduce your ideas on the gnucash-devel mailing list or on the #gnucash irc channel so they can be discussed by others. I'd prefer to avoid you doing additional work to find out in the end it has to be rejected for whatever reason.
Comment 7 Frank H. Ellenberger 2017-01-22 20:38:55 UTC
To split noise from real changes:

preparing patch:
 makepot to get a current pot file
 msgmerge the pot in your languages po file
 commit "Preparing patch: update ll.po from recent pot"

real work:
 edit po
 msgfmt -c --statistics LL.po
 commit "update of ll.po" preferable in line 3 with the statistics from msgfmt

Then this 2. would show only your changes, but both be published.
Comment 8 John Ralls 2018-06-29 23:53:33 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=777230. Please update any external references or bookmarks.