[kmymoney4] [Bug 360129] CSV Importer doesn't recognize sell operation in Polish

[NSLW via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=360129

NSLW  changed:

   What|Removed |Added

 Status|CONFIRMED   |RESOLVED
   Version Fixed In||4.8.0
 Resolution|--- |FIXED
  Latest Commit||http://commits.kde.org/kmym
   ||oney/59cd58a72ed13c6d69d8a9
   ||ebf8c8533e863ba55a

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kmymoney4] [Bug 360129] CSV Importer doesn't recognize sell operation in Polish

[NSLW via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=360129

--- Comment #18 from NSLW  ---
Created attachment 98417
  --> https://bugs.kde.org/attachment.cgi?id=98417=edit
Fill parameters of new investment profiles with translated strings

Additionally to previous patch, I recommend applying also this patch. It in
some kind moves [Profiles-New Profile###] template from
/usr/share/config/csvimporterrc to KMyMoney code, thus making it easy
translatable. Please revise it and apply to master branch.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kmymoney4] [Bug 360129] CSV Importer doesn't recognize sell operation in Polish

[NSLW via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=360129

NSLW  changed:

   What|Removed |Added

  Attachment #97870|0   |1
is obsolete||

--- Comment #17 from NSLW  ---
Created attachment 97970
  --> https://bugs.kde.org/attachment.cgi?id=97970=edit
Do not fetch from csvimporterrc if it's empty

Patch to fix this and similar issues. Please revise it and apply to master
branch.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kmymoney4] [Bug 360129] CSV Importer doesn't recognize sell operation in Polish

[NSLW via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=360129

--- Comment #16 from NSLW  ---
(In reply to allan from comment #15)
> > > I know no Polish financial terms.  If the Buy list contains a correct 
> > > Polish
> > > term, then that I presume was added by the translator.  A possible
> > > explanation is that at line 161 (in my version) of investprocessing.cpp 
> > > the
> > > following appears - "m_buyList += i18nc("verb", "buy");  //   
> > >   
> > > some basic entries in case rc file missing".  It may be that that was when
> > > the translation for Buy was added.  However, the next line is similar for
> > > the Sell operation, and that was not translated apparently.
> > 
> > All terms are and were translated.
> 
> In the rc file snippet you attached, only Buy and Reinvdiv have content.

That's right and that also shows that KMM has bug inside.
Please see translation progress of KMM to Polish to know that it is translated
fully 
http://l10n.kde.org/stats/gui/trunk-kf5/team/pl/extragear-office/

Please read also what I wrote you about the bug I'm seeing
> > After the patch my csvimporterrc is
> > filled out automatically with all the data whereas before the patch only
> > BuyParam and ReinvdivParam were filled out.

And finally please see into code of investprocessing.cpp

BuyParam and  ReinvdivParam are assigned like this

QStringList list = profilesGroup.readEntry("BuyParam", QStringList());
if (!list.isEmpty()) {
  m_buyList = list;
}


whereas other parameters and it includes SellParam is assigned like this


m_sellList = profilesGroup.readEntry("SellParam", QStringList());


and that's the reason why you don't see SellParam in rc file and not the
missing translation.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kmymoney4] [Bug 360129] CSV Importer doesn't recognize sell operation in Polish

[allan via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=360129

allan  changed:

   What|Removed |Added

 Status|UNCONFIRMED |CONFIRMED
 Ever confirmed|0   |1

--- Comment #15 from allan  ---
(In reply to NSLW from comment #14)
> (In reply to allan from comment #13)
> > What I'm not sure about just now is whether, once the user has chosen to
> > make a substitution, that new term is added to the resource file
> > automatically.  It's a few ears since I wrote that code.
> 
> That's worth checking. That might be a second bug.

I'll check.
> 
> > I know no Polish financial terms.  If the Buy list contains a correct Polish
> > term, then that I presume was added by the translator.  A possible
> > explanation is that at line 161 (in my version) of investprocessing.cpp the
> > following appears - "m_buyList += i18nc("verb", "buy");  // 
> > some basic entries in case rc file missing".  It may be that that was when
> > the translation for Buy was added.  However, the next line is similar for
> > the Sell operation, and that was not translated apparently.
> 
> All terms are and were translated.

In the rc file snippet you attached, only Buy and Reinvdiv have content.
> 
> > I would suggest that you include both noun and verb versions for Sell in
> > your resource file.
> 
> I already have them and it works for me.
> 
> > > As it is now was also misleading to me, to today although I'm an fresh
> > > KMyMoney user :) and translator, so maybe another suggestion will sound
> > > better to an layman:
> > > "Type of operation as in financial statement"
> > > 
> > That does sound more understandable, although it would have to be added to
> > every transaction type.
> 
> I think you're right, is there any problem for it to be added to every
> transaction type?

Only in terms of bloat.  However, if I create a single string, I can refer to
that instead of using the full version.

> 
> > > > So, apart from the fee query, are you now in business?
> > > 
> > > After manually editing csvimporterrc file, fee is imported just right for
> > > all operations. What do you mean by me being in business?
> > 
> > Apologies for that.  I'm extremely impressed by the general level of
> > knowledge of English shown by the users, and developers, and I forget myself
> > sometimes and lapse into the vernacular.  "being in business" is used to
> > indicate that a problem has been solved and normal service may be resumed. 
> > I hope it didn't cause offence.
> 
> No, none at all. You've suggested me a workaround and it worked for me as
> expected.
> 
> > So far as your suggested patch is concerned, I'm not sure of its purpose. 
> > The Subject is "[PATCH] Read operation's type explicitly as QStringList",
> > and in it you add a number of QStringList definitions, which are already
> > included in investprocessing.h, c. line 164.
> 
> I think that after my patch, every variable is defined anew locally, which I
> think is not clean way to do this and doesn't remove the source of the
> problem, but it works flawlessly.  After the patch my csvimporterrc is
> filled out automatically with all the data whereas before the patch only
> BuyParam and ReinvdivParam were filled out.
> I tell you what, I'm going to see into the problem once again soon and I
> will try to find its source, cause I really want it to see it fixed.
> 
> > Oh, and just as a post script, I've added "in Polish" to the bug heading, as
> > the import operation is otherwise as expected.
> 
> I would say "in non-English"

Well, we don't know of any other problem, and it seems to depend on
translation, so I think I'll stick with the current version.  It can always be
edited if needed in the future.

> 
> Łukasz

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kmymoney4] [Bug 360129] CSV Importer doesn't recognize sell operation in Polish

[NSLW via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=360129

--- Comment #14 from NSLW  ---
(In reply to allan from comment #13)
> What I'm not sure about just now is whether, once the user has chosen to
> make a substitution, that new term is added to the resource file
> automatically.  It's a few ears since I wrote that code.

That's worth checking. That might be a second bug.

> I know no Polish financial terms.  If the Buy list contains a correct Polish
> term, then that I presume was added by the translator.  A possible
> explanation is that at line 161 (in my version) of investprocessing.cpp the
> following appears - "m_buyList += i18nc("verb", "buy");  // 
> some basic entries in case rc file missing".  It may be that that was when
> the translation for Buy was added.  However, the next line is similar for
> the Sell operation, and that was not translated apparently.

All terms are and were translated.

> I would suggest that you include both noun and verb versions for Sell in
> your resource file.

I already have them and it works for me.

> > As it is now was also misleading to me, to today although I'm an fresh
> > KMyMoney user :) and translator, so maybe another suggestion will sound
> > better to an layman:
> > "Type of operation as in financial statement"
> > 
> That does sound more understandable, although it would have to be added to
> every transaction type.

I think you're right, is there any problem for it to be added to every
transaction type?

> > > So, apart from the fee query, are you now in business?
> > 
> > After manually editing csvimporterrc file, fee is imported just right for
> > all operations. What do you mean by me being in business?
> 
> Apologies for that.  I'm extremely impressed by the general level of
> knowledge of English shown by the users, and developers, and I forget myself
> sometimes and lapse into the vernacular.  "being in business" is used to
> indicate that a problem has been solved and normal service may be resumed. 
> I hope it didn't cause offence.

No, none at all. You've suggested me a workaround and it worked for me as
expected.

> So far as your suggested patch is concerned, I'm not sure of its purpose. 
> The Subject is "[PATCH] Read operation's type explicitly as QStringList",
> and in it you add a number of QStringList definitions, which are already
> included in investprocessing.h, c. line 164.

I think that after my patch, every variable is defined anew locally, which I
think is not clean way to do this and doesn't remove the source of the problem,
but it works flawlessly.  After the patch my csvimporterrc is filled out
automatically with all the data whereas before the patch only BuyParam and
ReinvdivParam were filled out.
I tell you what, I'm going to see into the problem once again soon and I will
try to find its source, cause I really want it to see it fixed.

> Oh, and just as a post script, I've added "in Polish" to the bug heading, as
> the import operation is otherwise as expected.

I would say "in non-English"

Łukasz

-- 
You are receiving this mail because:
You are the assignee for the bug.

[kmymoney4] [Bug 360129] CSV Importer doesn't recognize sell operation in Polish

[allan via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=360129

allan  changed:

   What|Removed |Added

Summary|CSV Importer doesn't|CSV Importer doesn't
   |recognize sell operation|recognize sell operation in
   ||Polish

--- Comment #13 from allan  ---
(In reply to NSLW from comment #11)
> (In reply to allan from comment #10)
> > (In reply to NSLW from comment #8)
> > > Created attachment 97709 [details]
> > > csvimporterrc snippet
> > > 
> > > I looked in csvimporterrc and found that
> > > 1) BuyParam=kup
> > > 
> > > "kup" is an verb, while "kupno" is noun. Maybe KMyMoney detects "kupno" by
> > > regular expression.  
> > 
> > It's just "type.contains(*it, Qt::CaseInsensitive", so fairly loose to
> > improve matching chance.  Do you see any problem with that?
> 
> It works with my Buy operations, but I would like it to work also with my
> Sell operations out of the box. 

I'm afraid that may be a tall order.  It will only work out of the box if the
file being imported actually uses the exact same translation as that of the
translator of the KMyMoney file.  Unfortunately, what tends to be found is that
different financial institutions use different terms to describe their
transaction type, in fact sometimes changing within the same file.

This is the reason I allowed for alternative terms to be added to the parameter
list in the resource file.  If the file being imported contains an unrecognised
type, the user will be informed and given the chance to substitute with the
standard KMyMoney transaction type.  If this should happen more than the odd
time, the user may add that unrecognised type to the appropriate list in the
resource file.

What I'm not sure about just now is whether, once the user has chosen to make a
substitution, that new term is added to the resource file automatically.  It's
a few ears since I wrote that code.

> I think that "type.contains(*it, Qt::CaseInsensitive)" is harmless here.
> 

> > > 2) SellParam=
> > > 
> > > after setting this parameter to "Sprzedaż" my sell operations are 
> > > recognized
> > > correctly, but why isn't that preset just like BuyParam? Maybe SellParam
> > > isn't preset because of last letter in "Sprzedaż", which is diacritic 
> > > letter.

I know no Polish financial terms.  If the Buy list contains a correct Polish
term, then that I presume was added by the translator.  A possible explanation
is that at line 161 (in my version) of investprocessing.cpp the following
appears - "m_buyList += i18nc("verb", "buy");  //   some
basic entries in case rc file missing".  It may be that that was when the
translation for Buy was added.  However, the next line is similar for the Sell
operation, and that was not translated apparently.

I would suggest that you include both noun and verb versions for Sell in your
resource file.

> > 
> > I would have to throw that one back to you, as I didn't edit that file. 
> > Might you not have been the translator then?
> 
> "Sprzedaż" is meant too look like this so no need to edit it. What I meant
> by above is: Could there be an sort of encoding problem in KMyMoney with
> that Polish letter, which in the end prevents positive recognition?
> 
> > > As another suggestion, i think it is good to expand context from "verb" 
> > > to "Action/Type as 
> > > recognized by CSV Importer"
> > 
> > Perhaps that would make sense as you are a user as well as a translator?  
> > I'm not sure that a non-investment-wise translator might understand?
> 
> As it is now was also misleading to me, to today although I'm an fresh
> KMyMoney user :) and translator, so maybe another suggestion will sound
> better to an layman:
> "Type of operation as in financial statement"
> 
That does sound more understandable, although it would have to be added to
every transaction type.

> > So, apart from the fee query, are you now in business?
> 
> After manually editing csvimporterrc file, fee is imported just right for
> all operations. What do you mean by me being in business?

Apologies for that.  I'm extremely impressed by the general level of knowledge
of English shown by the users, and developers, and I forget myself sometimes
and lapse into the vernacular.  "being in business" is used to indicate that a
problem has been solved and normal service may be resumed.  I hope it didn't
cause offence.

So far as your suggested patch is concerned, I'm not sure of its purpose.  The
Subject is "[PATCH] Read operation's type explicitly as QStringList", and in it
you add a number of QStringList definitions, which are already included in
investprocessing.h, c. line 164.

Oh, and just as a post script, I've added "in Polish" to the bug heading, as
the import operation is otherwise as expected.

Allan

-- 
You are receiving this mail