| Summary: | Problems with libalkimia - was Problems importing CSV files | ||
|---|---|---|---|
| Product: | [Applications] kmymoney | Reporter: | allan <agander93> |
| Component: | general | Assignee: | KMyMoney Devel Mailing List <kmymoney-devel> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | onet.cristian, richier7 |
| Priority: | NOR | ||
| Version: | 4.6.1 | ||
| Target Milestone: | --- | ||
| Platform: | Ubuntu | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
|
Description
allan
2012-06-19 10:54:14 UTC
Using tab as separator and comma as decimal both lines are handled correctly initially, the '-512' correctly showing as '-512,00'. KMM version 4.6.2 now imports the values correctly, However, 4.6.1 does not. Can you build 4.6.2 from source, or get it from Claydoh's repository? Within the csv plugin, all gets handled correctly in csvprocessing.cpp until line 469. tr.m_amount = m_trData.amount; a) m_trData.amount is "-512,00" but tr.m_amount produces "-51200/1" b) m_trData.amount is "-2478,25" but tr.m_amount produces "-247825/1". 4.6.2 handles this correctly. Correction. 4.6.2 has the same problem, but git definitely has the fix. Am I right in thinking that 4.6.2 is the current stable release and that this problem needs to fixed in it? Or, as it is fixed in git, no action is required here? I'm not sure if in general distros will update, but perhaps Claydoh might? I am lost: can you please elaborate what causes the problem as I don't see why it is a problem with libalkimia but fixed in KMyMoney git master. I am not saying that this can't be, but a pointer in the right direction to understand what's going on is certainly welcome. And if it is fixed in git master the question arises if that fix has been backported to the 4.6 branch. Sorry for causing the confusion. During debugging, I had traced the cause to the tr.m_amount = m_trData.amount statement and had found that tr.m_amount.decimalSeparator() produced a '.' when it should have been a ','. I tried a couple of 'fixes' which didn't work, and monitoring the libAlkimia ctor, I found it too was producing the wrong separator, decided I needed help, so changed the heading to include libalkimia, as I thought that was the problem area. Before submitting the revised bug entry, the original poster responded that he had now switched to 4.6.2 and still had the problem, and I then realised that the problem had been fixed in git rather than 4.6.2. I had been thinking the problem might lie with the change to the libAlkimia version, but now realised that wasn't the case. With further monitoring of the libalkimia ctor, I found the separator was correct during KMM startup, but changed as the csvplugin started, before it manipulated the decimal separator. I traced the problem ctors to the initialisation of three mymoneymoney instances. Casting these to libalkimia fixed them, but not the failing csv file. It was then I found that in git I had changed the tr.m_amount = m_trData.amount statements to tr.m_amount = MyMoneyMoney(m_trData.amount). I then submitted the bug entry with the confusing heading. To be fair to myself, it does say 'Problem with...', not 'Problem in...', and I did need to get some help. The change in git was introduced last November, related to the change to wizard structure, one of several changes. It was not back-ported. Git is definitely able to import the problem file, and the same change made to my local 4.6.1 also allows it to do the import successfully. So, what next? Getting back to my query in comment 3, should this be backported to 4.6.2? If so, I would welcome guidance on exactly how to do this. I would intend to change just this particular piece of code, which is very small, not to add the whole of the commit of which it was part, as the commit was for application to the wizard version of the plugin. It would also be helpful to know what must, or should, or should not be packported. Presumably, it would go back no further than the current stable release? That is, 4.6.2 and not also to 4.6.1? Can you provide the SHA1 of the fix in git master? If uncertain, a date/time of the commit is also welcome. (In reply to comment #7) > Can you provide the SHA1 of the fix in git master? If uncertain, a date/time > of the commit is also welcome. It's ff75edf07655f814de489e336d7f5c501a7540c0 ( 21 Nov.), part of several fixes as follow up to the first csvplugin wizard. Just a two-line change resolves the present issue, but I've noticed a similar need elsewhere, which appears not (yet) to be causing trouble. Presumably, I should attend to this myself in my next commit to git? I see that this fix has not been backported to the 4.6 branch (the file csvdialog.cpp is missing all together). So it's a matter of checking what is going on in the 4.6 series and fix it there. All you need to do is to switch to the 4.6 branch in your environment. Do that as follows: git checkout 4.6 git pull -r That should leave you with the latest that is available for 4.6. You know best what has been added to the importer since then. If you need further help, please ask on the devel mailinig list. (In reply to comment #9) > I see that this fix has not been backported to the 4.6 branch (the file > csvdialog.cpp is missing all together). So it's a matter of checking what is > going on in the 4.6 series and fix it there. That is because the 4.6 branch contains the first implementation of the csvimporter plugin while the master branch contains a re-written (wizard based) implementation. I guess since we are talking about a plugin we could backport the whole version from the master branch if it has been thoroughly tested but I'm not sure about that. (In reply to comment #10) > (In reply to comment #9) > > I see that this fix has not been backported to the 4.6 branch (the file > > csvdialog.cpp is missing all together). So it's a matter of checking what is > > going on in the 4.6 series and fix it there. > > That is because the 4.6 branch contains the first implementation of the > csvimporter plugin while the master branch contains a re-written (wizard > based) implementation. Yes, that is exactly the case. The fix for the current problem is very minor and I have implemented that now in 4.6.2. Going beyond that, I need to go through quite a few commits to see what might be relevant or important enough to backport, many being specific only to the wizard version. > I guess since we are talking about a plugin we could > backport the whole version from the master branch if it has been thoroughly > tested but I'm not sure about that. That's an interesting idea, but it is a biggish change, from the testing point of view, and I'm not sure it would be justified. It would certainly delay the fix to the current problem. Just let me know how to proceed. If you have only the fix against 4.6.2 for this problem then commit and push it into the 4.6 branch. *** Bug 303969 has been marked as a duplicate of this bug. *** Sorry to have duplicated a bug!! I have installed the 4.6.2 version on my computer (by : deb http://ppa.launchpad.net/claydoh/kmymoney2-kde4/ubuntu precise main ), but I have still the problem. (In reply to comment #14) > Sorry to have duplicated a bug!! No worries. > I have installed the 4.6.2 version on my computer (by : deb > http://ppa.launchpad.net/claydoh/kmymoney2-kde4/ubuntu precise main ), but I > have still the problem. The original 4.6.2 did have the bug, but anumber of fixes were back-ported from Git on 30 Jun. It might be worth contacting Claydoh and asking if he can possibly update from Git. Or, looking at Debian testing to see if they have updated their 4.6.2. Otherwise, I'm afraid it's a case of compiling from source, or waiting until...? @Thomas. Having commited the backported fixes, how do the distros find out about them/it? @Allen: we would have to release 4.6.3 so that they pick it up.. (In reply to comment #16) > @Allen: we would have to release 4.6.3 so that they pick it up.. Yes, I had a feeling we would need to do that. Well, having gone to the trouble of back-porting, I guess that needs to be done. By whom? I know you're busy. Is it something I could/should do? If so, how? I have compiled, now I have Version 4.6.90-1701953db7. The importation is OK now. Thanks for your help. (In reply to comment #18) > I have compiled, now I have Version 4.6.90-1701953db7. > The importation is OK now. > Thanks for your help. Glad you're OK now. (In reply to comment #17) > (In reply to comment #16) > > @Allen: we would have to release 4.6.3 so that they pick it up.. > > Yes, I had a feeling we would need to do that. Well, having gone to the > trouble of back-porting, I guess that needs to be done. By whom? I know > you're busy. Is it something I could/should do? If so, how? Releasing 4.6.3 would not stop the back-porting issue.as 4.6.3 will live on the 4.6 branch not in master. I will start the discussion of a release among those who have done it in the past: it always was a collaborative thing. As I understood this was fixed in alkimia. On 29/07/14 11:12, Cristian Oneț wrote: > https://bugs.kde.org/show_bug.cgi?id=302181 > > Cristian Oneț <onet.cristian@gmail.com> changed: > > What |Removed |Added > ---------------------------------------------------------------------------- > Resolution|--- |FIXED > Status|CONFIRMED |RESOLVED > > --- Comment #21 from Cristian Oneț <onet.cristian@gmail.com> --- > As I understood this was fixed in alkimia. > Not quite. It's a pretty rambling bug report, but the problem, which is resolved, was fixed by me in the CSV plugin. I added the "Problems *with* libalkimia" as I was seeking help with it, but isolated the problem to the plugin. Allan OK, but it was fixed so the status is correct. |