This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/
On April 7th, 2016, 7:59 p.m. UTC, *Christian David* wrote:
kmymoney/plugins/csvimport/investprocessing.cpp
<https://git.reviewboard.kde.org/r/127559/diff/1/?file=455181#file455181line1967>
(Diff revision 1)
1967
m_shrsinList = profilesGroup.readEntry("ShrsinParam",
QStringList());
1967 Łukasz
list = profilesGroup.readEntry("ShrsinParam", QStringList());
Should become |m_shrsinList =
profilesGroup.readEntry("ShrsinParam", m_shrsinList);|
The if() is very long and not needed here. However, I still do
not know if this is the issue. Also the |i18nc()s| from |init()|
could go here if the readSettings method is always called, which
I do not know either.
On April 8th, 2016, 1:52 p.m. UTC, *Allan Anderson* wrote:
Should become m_shrsinList =
profilesGroup.readEntry("ShrsinParam", m_shrsinList);
I'm not sure I understand this. The second parameter is the
default value to return if the key is not found. What does it
achieve in this case?
The if() is very long and not needed here.
There are several ifs around here, but I don't see an unduly
long one.
Also the i18nc()s from init() could go here if the
readSettings method is always called, which I do not know
either.
readSettings is called only once, from void
InvestProcessing::slotFileDialogClicked(), so that code could be
moved somewhere in void InvestProcessing::readSettings(), I think.
On April 8th, 2016, 4:36 p.m. UTC, *Łukasz Wojniłowicz* wrote:
Should become m_shrsinList =
profilesGroup.readEntry("ShrsinParam", m_shrsinList);
I compiled KMyMoney code according to your change for every
m_XXXList variable and ran my test case. Proposed line looks
neat but doesn't work for me. SellParam= etc. are empty in my
csvimporterrc after just created new profile.
readSettings is called only once, from void
InvestProcessing::slotFileDialogClicked(), so that code
could be moved somewhere in void
InvestProcessing::readSettings(), I think.
Please give a code and I'll test it.
I do not know the full conversation but I am pretty sure
this patch will not solve the issue. If something in the
newly created rc file is missing, the write method seems to
fail, not the read method.
My loose observation: Write method is called at the end of
importing and read method is called after creating new importing
profile for investment.
On April 8th, 2016, 7:34 p.m. UTC, *Christian David* wrote:
I do not know the full conversation but I am pretty sure
this patch will not solve the issue. If something in the
newly created rc file is missing, the write method seems to
fail, not the read method.
I withdrew this idea. You should not waste your time with it.
I compiled KMyMoney code according to your change for every
m_XXXList variable and ran my test case. Proposed line looks
neat but doesn't work for me. SellParam= etc. are empty in
my csvimporterrc after just created new profile.
You are right. The code recomended by me has a different
behaviour. However, now I doubt that anything I wrote was
actually helpfull. I just briefly inspected the code – now I see
that is more complex than I thougt. So my recomendations are
based on insufficent knowledge. Due to the description in the
bug report I still think there is a high chance that the issue
is in the write function.
Due to the description in the bug report I still think there is a high
chance that the issue is in the write function.
And you may be right, look what I've found. New entry of importing
profile in |$HOME/.kde/share/config/csvimporterrc| is created in
csvdialog.cpp by following routine
void CSVDialog::createProfile(QString newName)
{
KSharedConfigPtr config=
KSharedConfig::openConfig(KStandardDirs::locateLocal("config","csvimporterrc"));
KConfigGroupbankProfilesGroup(config,"BankProfiles");
bankProfilesGroup.writeEntry("BankNames", m_profileList);
bankProfilesGroup.config()->sync();
KConfigGroupbankGroup(config,"BankProfiles");
QString txt= "Profiles-" + newName;
KConfigGroupprofilesGroup(config,"Profiles-New Profile###");
KSharedConfigPtr configBackup=
KSharedConfig::openConfig(KStandardDirs::locate("config","csvimporterrc"));
KConfigGroupbkprofilesGroup(configBackup,"Profiles-New Profile###");
KConfigGroupnewProfilesGroup(config, txt);
bkprofilesGroup.copyTo(&newProfilesGroup);
newProfilesGroup.writeEntry("FileType", m_fileType);
if (m_fileType== "Invest") {
m_investProcessing->m_shrsinList=
bkprofilesGroup.readEntry("ShrsinParam", QStringList());
newProfilesGroup.writeEntry("ShrsinParam",
m_investProcessing->m_shrsinList);
m_investProcessing->m_divXList= bkprofilesGroup.readEntry("DivXParam",
QStringList());
newProfilesGroup.writeEntry("DivXParam", m_investProcessing->m_divXList);
m_investProcessing->m_intIncList=
bkprofilesGroup.readEntry("IntIncParam", QStringList());
newProfilesGroup.writeEntry("IntIncParam",
m_investProcessing->m_intIncList);
m_investProcessing->m_brokerageList=
bkprofilesGroup.readEntry("BrokerageParam", QStringList());
newProfilesGroup.writeEntry("BrokerageParam",
m_investProcessing->m_brokerageList);
m_investProcessing->m_reinvdivList=
bkprofilesGroup.readEntry("ReinvdivParam", QStringList());
newProfilesGroup.writeEntry("ReinvdivParam",
m_investProcessing->m_reinvdivList);
m_investProcessing->m_buyList= bkprofilesGroup.readEntry("BuyParam",
QStringList());
newProfilesGroup.writeEntry("BuyParam", m_investProcessing->m_buyList);
m_investProcessing->m_sellList= bkprofilesGroup.readEntry("SellParam",
QStringList());
newProfilesGroup.writeEntry("SellParam", m_investProcessing->m_sellList);
m_investProcessing->m_removeList=
bkprofilesGroup.readEntry("RemoveParam", QStringList());
newProfilesGroup.writeEntry("RemoveParam",
m_investProcessing->m_removeList);
}
newProfilesGroup.config()->sync();
}
According to above routine, new entries in
|$HOME/.kde/share/config/csvimporterrc| supposedly should be created
from template entry (Profiles-New Profile###) in
|$USR/share/config/csvimporterrc| (read-only-file). Moreover all
m_XXXList are defined here also and supposedly shouldn't be empty
because template entry isn't empty.
On my system (Fedora 23, KDE Frameworks 5.21.0, Qt 5.5.1) it doesn't
work though. Snippets of code:
|KStandardDirs::locateLocal("config", "csvimporterrc")| responsible for
locating |$HOME/.kde/share/config/csvimporterrc|
and |KStandardDirs::locate("config", "csvimporterrc")| responsible for
locating |$USR/share/config/csvimporterrc|
both point to the same file and that is
|$HOME/.kde/share/config/csvimporterrc|
Config file |$HOME/.kde/share/config/csvimporterrc| hasn't got
"Profiles-New Profile###" so empty values are defined in m_XXXList and
in new entry.
Do |KStandardDirs::locate("config", "csvimporterrc")| works correctly on
my system? I think, yes.
According to http://api.kde.org |KStandardDirs::locate| is convenience
function for |KGlobal::dirs()->findResource(type, filename)| where in
our case, type is "config" and filename is "csvimporterrc". That
function will
The filename should be a filename relative to the base dir for
resources. So is a way to get the path to csvimporterrc to
findResource("config", "csvimporterrc"). KStandardDirs will then
look into the subdir config of all elements of all prefixes
($KDEDIRS) for a file csvimporterrc and return the path to the first
one it finds (e.g. $HOME/.kde/share/config/csvimporterrc). You can
use the program kde4-config to list all resource types.
My |kde4-config --path config| gives me
|$HOME/.kde/share/config/:/etc/kde/:$USR/share/kde-settings/kde-profile/default/share/config/:$USR/share/config/|
Notice that |$HOME/.kde/share/config/| is before |$USR/share/config/| so
it will be quered first by |KStandardDirs::locate| and according to
http://api.kde.org |KStandardDirs::locate| will return full path to the
first one file it finds and in case of "csvimporterrc" it always will be
|$HOME/.kde/share/config/csvimporterrc|
My thoughts: 1) |KStandardDirs| is deprecated and should be replaced
with |QStandardPaths| but I don't think it can be done now since I
suppose that KMyMoney is compiled against < Qt 5 now. 2) csvimporterrc
in |$USR/share/config/| could have unique name, so |KStandardDirs| won't
confuse it with the one in local dir but I don't think it's the best
idea because "Profiles-New Profile###" is preset with all English types
of operations and that won't work for non-English users 3) I still do
believe that my original path is good here, because: it won't harm not
to fetch empty values from csvimporterrc into KMyMoney, it's reported to
work in non-English environment, it shouldn't break usage in English
environment
What are your opinions?
@Allan You've said that your csvimporterrc is initiated with all
required values, oppositely to mine; what is your system configuration
(OS, KDE, Qt)?
- Łukasz
