Re: Review Request 129790: Add Capital Gains report

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129790/#review101864
---



Just a query, as I have not studied the code, but are you able to handle the 
different requirements of different countries?

- Allan Anderson


On Jan. 7, 2017, 7:50 p.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129790/
> ---
> 
> (Updated Jan. 7, 2017, 7:50 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 250471
> http://bugs.kde.org/show_bug.cgi?id=250471
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> This new report presents capital gains calculated using FIFO method and is 
> displayed in table.
> 
> 
> Diffs
> -
> 
>   kmymoney/mymoney/mymoneyreport.h 7397a0f 
>   kmymoney/mymoney/mymoneyreport.cpp 642145a 
>   kmymoney/reports/listtable.cpp 72b605f 
>   kmymoney/reports/querytable.h 7e2bfa1 
>   kmymoney/reports/querytable.cpp e44f74c 
>   kmymoney/views/kreportsview.cpp c9c9a12 
> 
> Diff: https://git.reviewboard.kde.org/r/129790/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128874: Free QFileDialog memory

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128874/#review101573
---



Did you get a solution to this problem?  The reason I ask is that there is 
still a problem lurking in this area.
Having selected a profile, I then go to select the file to import. Having got 
to the file selector, if I then decide instead to reverse to get a different 
profile, I cancel the file selector and am returned to the intro profile 
screen.  If I cancel that too, when KMM is terminated, it aborts - Segmentation 
fault (core dumped).  I've spent quite a bit of time on this, making sure 
everything that should be deleted, is deleted, but don't yet have a solution.  
As in this review, the Fileselect dialog is the crux, commenting it out avoids 
the abort.

- Allan Anderson


On Sept. 16, 2016, 4:38 p.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128874/
> ---
> 
> (Updated Sept. 16, 2016, 4:38 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> After closing CSV Importer in the middle and then KMyMoney, I get "The 
> program has unexpectedly finished".
> The problem doesn't occur if CSV Importer goes all way through to the last 
> page; then I can go back and close it wherever I want.
> If I comment out this line, there is no problem at all.
> ```c++
>   QPointer dialog = new QFileDialog(this, QString(),
>  fileInfo.absoluteFilePath(),
>  i18n("*.csv *.PRN *.txt | 
> CSV Files\n *|All files"));
> ```
> Memory on which dialog pointed wasn't deleted in the method and it obviously 
> need to be deleted, but the problem remains. Does anyone know how to prevent 
> QtCreator from showing "The program has unexpectedly finished" here?
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/csvwizard.h ecec5b0 
>   kmymoney/plugins/csvimport/csvwizard.cpp b576dea 
> 
> Diff: https://git.reviewboard.kde.org/r/128874/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128878: Add slotSaveAsQIFClicked to CSV Importer

[Allan Anderson]

> On Sept. 11, 2016, 10:09 a.m., Allan Anderson wrote:
> > It was actually on my todo list to remove the QIF facility as it no longer 
> > had any purpose.  It was a relict from early days before CSV import became 
> > fully established.
> > I had indicated this on the lists several times without receiving any 
> > protests.  It's possible, I suppose, that on implementation, a non-lister 
> > might discover that a much needed feature had been removed.  I would still 
> > have gone ahead, but the change would have had to be reverted in that 
> > circumstance.
> > Perhaps you see a need?
> 
> Łukasz Wojniłowicz wrote:
> I didn't know that you wanted to remove QIF facility. I don't use it 
> personally. Initially I wanted to move it as separate CSV->QIF converter but 
> it would involve the same steps you do during CSV import, so I left it where 
> it is.
>     I think defeaturing CSV importer of QIF converter would be loss of work.
> 
> Allan Anderson wrote:
> It's certainly not causing any harm, that I'm aware of.  It was purely to 
> remove clutter.  No hard views, either way, though.
> 
> Łukasz Wojniłowicz wrote:
> We can hide it deep with an option through recent configuration dialog to 
> remove clutter, if all you devs think it would be a good idea :)
> 
> Allan, do you use CSV Importer from master branch? Lots of code have been 
> changed recently and I'm little bit concerned about usablity in all cases.

I'm afraid I have not had a lot of time lately, what with my hospital 
appointments, dental troubles and now my wife was admitted to hospital 10 days 
ago.  Also, I've been waiting on my distro, and as it happens only two days ago 
it released a new version.
As soon as I can find time, I'll try to upgrade, etc.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128878/#review99081
---


On Sept. 10, 2016, 4:35 p.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128878/
> ---
> 
> (Updated Sept. 10, 2016, 4:35 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> 1) It's now possible to save qif file with investments,
> 2) If account is available, then it will be added to qif file,
> 3) If type of import is available, then it will be added to qif file,
> 4) Handled canceling of QFileDialog,
> 5) QFileDialog saves only .qif files now,
> 6) Date format is hardcoded to MM/dd/, because it is so in files, that I 
> saw.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/csvdialog.h 3cedd92 
>   kmymoney/plugins/csvimport/csvdialog.cpp 556d1c5 
>   kmymoney/plugins/csvimport/csvwizard.h 48c15ea 
>   kmymoney/plugins/csvimport/csvwizard.cpp fcf73fd 
>   kmymoney/plugins/csvimport/investprocessing.h 6ca2e53 
>   kmymoney/plugins/csvimport/investprocessing.cpp 7499b10 
> 
> Diff: https://git.reviewboard.kde.org/r/128878/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128878: Add slotSaveAsQIFClicked to CSV Importer

[Allan Anderson]

> On Sept. 11, 2016, 10:09 a.m., Allan Anderson wrote:
> > It was actually on my todo list to remove the QIF facility as it no longer 
> > had any purpose.  It was a relict from early days before CSV import became 
> > fully established.
> > I had indicated this on the lists several times without receiving any 
> > protests.  It's possible, I suppose, that on implementation, a non-lister 
> > might discover that a much needed feature had been removed.  I would still 
> > have gone ahead, but the change would have had to be reverted in that 
> > circumstance.
> > Perhaps you see a need?
> 
> Łukasz Wojniłowicz wrote:
> I didn't know that you wanted to remove QIF facility. I don't use it 
> personally. Initially I wanted to move it as separate CSV->QIF converter but 
> it would involve the same steps you do during CSV import, so I left it where 
> it is.
> I think defeaturing CSV importer of QIF converter would be loss of work.

It's certainly not causing any harm, that I'm aware of.  It was purely to 
remove clutter.  No hard views, either way, though.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128878/#review99081
---


On Sept. 10, 2016, 4:35 p.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128878/
> ---
> 
> (Updated Sept. 10, 2016, 4:35 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> 1) It's now possible to save qif file with investments,
> 2) If account is available, then it will be added to qif file,
> 3) If type of import is available, then it will be added to qif file,
> 4) Handled canceling of QFileDialog,
> 5) QFileDialog saves only .qif files now,
> 6) Date format is hardcoded to MM/dd/, because it is so in files, that I 
> saw.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/csvdialog.h 3cedd92 
>   kmymoney/plugins/csvimport/csvdialog.cpp 556d1c5 
>   kmymoney/plugins/csvimport/csvwizard.h 48c15ea 
>   kmymoney/plugins/csvimport/csvwizard.cpp fcf73fd 
>   kmymoney/plugins/csvimport/investprocessing.h 6ca2e53 
>   kmymoney/plugins/csvimport/investprocessing.cpp 7499b10 
> 
> Diff: https://git.reviewboard.kde.org/r/128878/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128878: Add slotSaveAsQIFClicked to CSV Importer

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128878/#review99081
---



It was actually on my todo list to remove the QIF facility as it no longer had 
any purpose.  It was a relict from early days before CSV import became fully 
established.
I had indicated this on the lists several times without receiving any protests. 
 It's possible, I suppose, that on implementation, a non-lister might discover 
that a much needed feature had been removed.  I would still have gone ahead, 
but the change would have had to be reverted in that circumstance.
Perhaps you see a need?

- Allan Anderson


On Sept. 10, 2016, 4:35 p.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128878/
> ---
> 
> (Updated Sept. 10, 2016, 4:35 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> 1) It's now possible to save qif file with investments,
> 2) If account is available, then it will be added to qif file,
> 3) If type of import is available, then it will be added to qif file,
> 4) Handled canceling of QFileDialog,
> 5) QFileDialog saves only .qif files now,
> 6) Date format is hardcoded to MM/dd/, because it is so in files, that I 
> saw.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/csvdialog.h 3cedd92 
>   kmymoney/plugins/csvimport/csvdialog.cpp 556d1c5 
>   kmymoney/plugins/csvimport/csvwizard.h 48c15ea 
>   kmymoney/plugins/csvimport/csvwizard.cpp fcf73fd 
>   kmymoney/plugins/csvimport/investprocessing.h 6ca2e53 
>   kmymoney/plugins/csvimport/investprocessing.cpp 7499b10 
> 
> Diff: https://git.reviewboard.kde.org/r/128878/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128511: Move displayLine to CSVWizard

[Allan Anderson]

> On July 24, 2016, 10:54 a.m., Thomas Baumgart wrote:
> > Again, just visual inspection. Looks good to me besides the two little 
> > nitpicks. Someone with more knowledge about the functionality should check 
> > as well.

If that includes me, then I agree.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128511/#review97792
---


On July 24, 2016, 9:07 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128511/
> ---
> 
> (Updated July 24, 2016, 9:07 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Purpose of displayLine in tableWidget shouldn't differ whether it's
> investment or banking statement, so take of responsibilities of parsing
> data into columns and creating memo field from displayLine and put them
> into separate functions.
> 
> In csvdialog.cpp and investprocessing.cpp, I introduced createMemoField (it 
> simplifies memo concatenating and allows to remove some redundant boolean 
> variables), which in both places looks the same. I think it will be easy to 
> move it into csvwizard.cpp after trying to move e.g. readFile there, so that 
> will be next target.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/csvdialog.h 69cca6e 
>   kmymoney/plugins/csvimport/csvdialog.cpp fa70b04 
>   kmymoney/plugins/csvimport/csvwizard.h 28eea62 
>   kmymoney/plugins/csvimport/csvwizard.cpp 7aee196 
>   kmymoney/plugins/csvimport/investprocessing.h 38f622c 
>   kmymoney/plugins/csvimport/investprocessing.cpp 328a79b 
> 
> Diff: https://git.reviewboard.kde.org/r/128511/diff/
> 
> 
> Testing
> ---
> 
> Banking and investment statement CSV imports; with and without setup.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128061: Fix precision of split value

[Allan Anderson]

> On June 2, 2016, 1 p.m., Allan Anderson wrote:
> > This is not a good area for me, and takes me out of my comfort zone.
> > However, I've been having a look at this, and there are one or two points, 
> > which may or may not be relevant.
> > Firstly, the file attached to https://bugs.kde.org/show_bug.cgi?id=303026 
> > does illustrate the problem.
> > Secondly, the sell transaction shows a missing assignment of "ACME 0.005".
> > Next, I edited it to remove the fee split, to simplify things a bit. It 
> > still shows the missing assignment.
> > Looking now at the void MyMoneyFile::fixSplitPrecision(MyMoneyTransaction& 
> > t) const routine, and specifically at the last line,
> > "(*its).setValue((*its).value().convertDenominator(fraction).canonicalize());"
> > For the first split, I get "16180.52", but with the canonicalize() 
> > removed, I get "16180.525000".
> > The second split gives "-16180.525000".
> > Whether or not this is relevant, I really don't know and an expert opinion 
> > is needed, I think.

I think perhaps I should have mentioned that with 
MyMoneyFile::fixSplitPrecision() bypassed, this file imports correctly, and the 
same details may be entered manually with no error.
Also, I've just noticed that even where the transaction now shows no error, the 
consistency check reports that it has fixed the error.  This it does with
"if (t.splitSum().isZero()) {
s.setShares(s.value());
}"
Using similar code in MyMoneyFile::fixSplitPrecision() appears to resolve the 
problem in this bug.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128061/#review96155
---


On May 30, 2016, 6:10 p.m., Thomas Baumgart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128061/
> ---
> 
> (Updated May 30, 2016, 6:10 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 345655
> http://bugs.kde.org/show_bug.cgi?id=345655
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> The problem described in bug 345655 is caused by a fraction of the splits 
> value that is larger than the fraction of the accounts currency. This leads 
> to the effect, that values are displayed with correct value but the 
> underlying value used for calculations is not equal to the amount displayed. 
> This will end up in KMyMoney presenting the user strange messages like 
> 'Unassigned value of 0.00' which do not make sense. The difference is in 
> reality between 0 and 0.01, e.g. 0.008.
> 
> The attached patch resolves this problem by rounding the numbers to the 
> correct fraction of the referrenced account.
> 
> 
> Diffs
> -
> 
>   kmymoney/mymoney/mymoneyfile.h 0fd558b 
>   kmymoney/mymoney/mymoneyfile.cpp 568675c 
>   kmymoney/mymoney/mymoneyfiletest.h 657ed39 
>   kmymoney/mymoney/mymoneyfiletest.cpp 810b15f 
> 
> Diff: https://git.reviewboard.kde.org/r/128061/diff/
> 
> 
> Testing
> ---
> 
> Test case is part of the patch. It uses the values provided in the sample 
> attached to the bug 345655 entry.
> 
> 
> Thanks,
> 
> Thomas Baumgart
> 
>

Re: Review Request 128061: Fix precision of split value

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128061/#review96155
---



This is not a good area for me, and takes me out of my comfort zone.
However, I've been having a look at this, and there are one or two points, 
which may or may not be relevant.
Firstly, the file attached to https://bugs.kde.org/show_bug.cgi?id=303026 does 
illustrate the problem.
Secondly, the sell transaction shows a missing assignment of "ACME 0.005".
Next, I edited it to remove the fee split, to simplify things a bit. It still 
shows the missing assignment.
Looking now at the void MyMoneyFile::fixSplitPrecision(MyMoneyTransaction& t) 
const routine, and specifically at the last line,
"(*its).setValue((*its).value().convertDenominator(fraction).canonicalize());"
For the first split, I get "16180.52", but with the canonicalize() removed, 
I get "16180.525000".
The second split gives "-16180.525000".
Whether or not this is relevant, I really don't know and an expert opinion is 
needed, I think.

- Allan Anderson


On May 30, 2016, 6:10 p.m., Thomas Baumgart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128061/
> ---
> 
> (Updated May 30, 2016, 6:10 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 345655
> http://bugs.kde.org/show_bug.cgi?id=345655
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> The problem described in bug 345655 is caused by a fraction of the splits 
> value that is larger than the fraction of the accounts currency. This leads 
> to the effect, that values are displayed with correct value but the 
> underlying value used for calculations is not equal to the amount displayed. 
> This will end up in KMyMoney presenting the user strange messages like 
> 'Unassigned value of 0.00' which do not make sense. The difference is in 
> reality between 0 and 0.01, e.g. 0.008.
> 
> The attached patch resolves this problem by rounding the numbers to the 
> correct fraction of the referrenced account.
> 
> 
> Diffs
> -
> 
>   kmymoney/mymoney/mymoneyfile.h 0fd558b 
>   kmymoney/mymoney/mymoneyfile.cpp 568675c 
>   kmymoney/mymoney/mymoneyfiletest.h 657ed39 
>   kmymoney/mymoney/mymoneyfiletest.cpp 810b15f 
> 
> Diff: https://git.reviewboard.kde.org/r/128061/diff/
> 
> 
> Testing
> ---
> 
> Test case is part of the patch. It uses the values provided in the sample 
> attached to the bug 345655 entry.
> 
> 
> Thanks,
> 
> Thomas Baumgart
> 
>

Re: Review Request 128061: Fix precision of split value

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128061/#review96153
---



>From the list -
https://bugs.kde.org/show_bug.cgi?id=345655

--- Comment #18 from allan <agande...@gmail.com> ---
Referring back to comments #3, #4,  #5, and
https://bugs.kde.org/show_bug.cgi?id=303026, does that issue now need 
revisiting?

--- Comment #19 from Thomas Baumgart <tbaumg...@kde.org> ---
Allan, good point. The modification is almost identical, though it differs a
bit. The rounding problem caught in https://bugs.kde.org/show_bug.cgi?id=303026
could still happen. Can you please comment on reviewboard so that we can think
of how to deal with the situation? It would be nice if you can provide me with
a testcase (probably just different values) that fails.

- Allan Anderson


On May 30, 2016, 6:10 p.m., Thomas Baumgart wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128061/
> ---
> 
> (Updated May 30, 2016, 6:10 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 345655
> http://bugs.kde.org/show_bug.cgi?id=345655
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> The problem described in bug 345655 is caused by a fraction of the splits 
> value that is larger than the fraction of the accounts currency. This leads 
> to the effect, that values are displayed with correct value but the 
> underlying value used for calculations is not equal to the amount displayed. 
> This will end up in KMyMoney presenting the user strange messages like 
> 'Unassigned value of 0.00' which do not make sense. The difference is in 
> reality between 0 and 0.01, e.g. 0.008.
> 
> The attached patch resolves this problem by rounding the numbers to the 
> correct fraction of the referrenced account.
> 
> 
> Diffs
> -
> 
>   kmymoney/mymoney/mymoneyfile.h 0fd558b 
>   kmymoney/mymoney/mymoneyfile.cpp 568675c 
>   kmymoney/mymoney/mymoneyfiletest.h 657ed39 
>   kmymoney/mymoney/mymoneyfiletest.cpp 810b15f 
> 
> Diff: https://git.reviewboard.kde.org/r/128061/diff/
> 
> 
> Testing
> ---
> 
> Test case is part of the patch. It uses the values provided in the sample 
> attached to the bug 345655 entry.
> 
> 
> Thanks,
> 
> Thomas Baumgart
> 
>

Re: Review Request 127712: Bug 360747 CSV Importer detects more columns than are assigned

[Allan Anderson]

> On April 27, 2016, 7:11 p.m., Cristian Oneț wrote:
> > kmymoney/plugins/csvimport/investprocessing.cpp, line 858
> > 
> >
> > Is this local variable really necessary?

I had noticed this.  Whilst it is used, I suspect that it could be replaced by 
m_parse, with possible minor adjustment, but I can't test, I'm afraid.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127712/#review94911
---


On April 22, 2016, 6:56 p.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127712/
> ---
> 
> (Updated April 22, 2016, 6:56 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360747
> http://bugs.kde.org/show_bug.cgi?id=360747
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360747. Current routine doesn't calculate columns well when
> FieldDelimiter=DecimalSymbol. parseLine() from csvutil.cpp does it
> properly.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 73e4e4d 
> 
> Diff: https://git.reviewboard.kde.org/r/127712/diff/
> 
> 
> Testing
> ---
> 
> 1) CSV file where FieldDelimiter=DecimalSymbol=comma (Test file from bug 
> #360747)
> 2) CSV file where FieldDelimiter=comma and DecimalSymbol=dot
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

[Allan Anderson]

> On April 7, 2016, 7:59 p.m., Christian David wrote:
> > kmymoney/plugins/csvimport/investprocessing.cpp, line 1967
> > <https://git.reviewboard.kde.org/r/127559/diff/1/?file=455181#file455181line1967>
> >
> > 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.
> 
> 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.
> 
> Ł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.
> 
> 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.
> 
> Łukasz Wojniłowicz wrote:
> 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
> 
> ```c++
> void CSVDialog::createProfile(QString newName)
> {
>   KSharedConfigPtr  config = 
> KSharedConfig::openConfig(KStandardDirs::locateLocal("config", 
> "csvimporterrc"));
>   KConfigGroup bankProfilesGroup(config, "BankProfiles");
> 
>   bankProfilesGroup.writeEntry("BankNames", m_profileList);
>   bankProfilesGroup.config()->sync();
> 
>   KConfigGroup bankGroup(config, "BankProfiles");
>   QString txt = "Profiles-" + newName;
> 
>   KConfigGroup profilesGroup(config, "Profiles-New Profile###");
> 
>   KSharedConfigPtr  configBackup = 
> KSharedConfig::openConfig(KStandardDirs::locate("config", "csvimporterrc"));
>   KConfigGroup bkprofilesGroup(configBackup, "Profiles

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

[Allan Anderson]

> On April 7, 2016, 7:59 p.m., Christian David wrote:
> > kmymoney/plugins/csvimport/investprocessing.cpp, line 1967
> > <https://git.reviewboard.kde.org/r/127559/diff/1/?file=455181#file455181line1967>
> >
> > 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.
> 
> 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.
> 
> Ł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.
> 
> 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.
> 
> Łukasz Wojniłowicz wrote:
> 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
> 
> ```c++
> void CSVDialog::createProfile(QString newName)
> {
>   KSharedConfigPtr  config = 
> KSharedConfig::openConfig(KStandardDirs::locateLocal("config", 
> "csvimporterrc"));
>   KConfigGroup bankProfilesGroup(config, "BankProfiles");
> 
>   bankProfilesGroup.writeEntry("BankNames", m_profileList);
>   bankProfilesGroup.config()->sync();
> 
>   KConfigGroup bankGroup(config, "BankProfiles");
>   QString txt = "Profiles-" + newName;
> 
>   KConfigGroup profilesGroup(config, "Profiles-New Profile###");
> 
>   KSharedConfigPtr  configBackup = 
> KSharedConfig::openConfig(KStandardDirs::locate("config", "csvimporterrc"));
>   KConfigGroup bkprofilesGroup(configBackup, "Profiles

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

[Allan Anderson]

> On April 7, 2016, 7:59 p.m., Christian David wrote:
> > kmymoney/plugins/csvimport/investprocessing.cpp, line 1967
> > 
> >
> > 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.

> 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.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/#review94397
---


On April 3, 2016, 4:45 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127559/
> ---
> 
> (Updated April 3, 2016, 4:45 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360129
> http://bugs.kde.org/show_bug.cgi?id=360129
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360129. During creation of new investment statement
> template, transaction types are initialized in
> investprocessing.cpp, but then are overridden with empty fields
> from profile that was just created in csvimporterrc which results
> in every non-buy transaction unrecognized during the import.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 3879819 
> 
> Diff: https://git.reviewboard.kde.org/r/127559/diff/
> 
> 
> Testing
> ---
> 
> Tested using financial statement from bug #360129.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

[Allan Anderson]

> On April 3, 2016, 12:37 p.m., Allan Anderson wrote:
> >
> 
> Allan Anderson wrote:
> We have had discussons about this in the bug report.  I think you are 
> confusing the issue by stating your experience (using the Polish translation) 
> as a general issue.  For me, if I delete the csvimporterrc file and then 
> initiate loading of an investment CSV, a new rc file is created containing 
> default values for all investment types. 
> From the extract of your rc file in the bug report, several investment 
> types have empty fields, although the Buy has an entry.
> 
> Łukasz Wojniłowicz wrote:
> For me, if I delete the csvimporterrc file and then initiate loading of 
> an investment CSV, a new rc file is created containing default value only for 
> Buy and Reinvest Dividend entry.
> I don't understand the confusion you're talking; this patch helps me. Do 
> you say it's wrong?

Just to let you know I'm still here.  Have had major system problems, that I'm 
still recovering from.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/#review94211
---


On April 3, 2016, 4:45 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127559/
> ---
> 
> (Updated April 3, 2016, 4:45 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360129
> http://bugs.kde.org/show_bug.cgi?id=360129
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360129. During creation of new investment statement
> template, transaction types are initialized in
> investprocessing.cpp, but then are overridden with empty fields
> from profile that was just created in csvimporterrc which results
> in every non-buy transaction unrecognized during the import.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 3879819 
> 
> Diff: https://git.reviewboard.kde.org/r/127559/diff/
> 
> 
> Testing
> ---
> 
> Tested using financial statement from bug #360129.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

[Allan Anderson]

> On April 3, 2016, 12:37 p.m., Allan Anderson wrote:
> >

We have had discussons about this in the bug report.  I think you are confusing 
the issue by stating your experience (using the Polish translation) as a 
general issue.  For me, if I delete the csvimporterrc file and then initiate 
loading of an investment CSV, a new rc file is created containing default 
values for all investment types. 
>From the extract of your rc file in the bug report, several investment types 
>have empty fields, although the Buy has an entry.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/#review94211
---


On April 3, 2016, 4:45 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127559/
> ---
> 
> (Updated April 3, 2016, 4:45 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360129
> http://bugs.kde.org/show_bug.cgi?id=360129
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360129. During creation of new investment statement
> template, transaction types are initialized in
> investprocessing.cpp, but then are overridden with empty fields
> from profile that was just created in csvimporterrc which results
> in every non-buy transaction unrecognized during the import.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 3879819 
> 
> Diff: https://git.reviewboard.kde.org/r/127559/diff/
> 
> 
> Testing
> ---
> 
> Tested using financial statement from bug #360129.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 127559: BUG 360129 Do not fetch from csvimporterrc if it's empty

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127559/#review94211
---



- Allan Anderson


On April 3, 2016, 4:45 a.m., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127559/
> ---
> 
> (Updated April 3, 2016, 4:45 a.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Bugs: 360129
> http://bugs.kde.org/show_bug.cgi?id=360129
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Fixes bug #360129. During creation of new investment statement
> template, transaction types are initialized in
> investprocessing.cpp, but then are overridden with empty fields
> from profile that was just created in csvimporterrc which results
> in every non-buy transaction unrecognized during the import.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/investprocessing.cpp 3879819 
> 
> Diff: https://git.reviewboard.kde.org/r/127559/diff/
> 
> 
> Testing
> ---
> 
> Tested using financial statement from bug #360129.
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 124815: BUG:347166 "Price/share" field on investment transaction entry form is mislabeled

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124815/
---

(Updated March 13, 2016, 11:30 a.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Bugs: 347166
http://bugs.kde.org/show_bug.cgi?id=347166


Repository: kmymoney


Description
---

Fix for "Price/share" field on investment transaction entry form is mislabeled.


Diffs
-

  kmymoney/dialogs/investactivities.h d2e2a76 
  kmymoney/dialogs/investactivities.cpp 29c7957 
  kmymoney/dialogs/investtransactioneditor.cpp 882e5a0 

Diff: https://git.reviewboard.kde.org/r/124815/diff/


Testing
---

Create and edit numerous relevant transactions.


Thanks,

Allan Anderson

Re: Review Request 126875: Combined the questions from TransactionMatcher::match and MyMoneyStatementReader::askUserToEnterScheduleForMatching

[Allan Anderson]

> On Jan. 24, 2016, 9:02 p.m., Christian David wrote:
> > kmymoney/converter/mymoneystatementreader.cpp, line 1458
> > <https://git.reviewboard.kde.org/r/126875/diff/1/?file=439594#file439594line1458>
> >
> > Translated strings should not be combined, as it confuses the 
> > translators. It is better to create two long strings using i18np() although 
> > this duplicates the text (which is usually bad).
> 
> Artur Puzio wrote:
> But the i18np() is for plurals. What we can do is change the previous 
> string only slitly to add the variable that will contain `"The transaction 
> dates are one day apart."` or `""`.
> 
> Christian David wrote:
> You could use something like
> 
> i18n("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %1"
>  "Transaction: %2 %3"
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  scheduleName, splitValue, payeeName);
>   
> and
> i18np("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>  "The transaction dates are one day 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  "KMyMoney has found a scheduled 
> transaction which matches an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>      "The transaction dates are %1 days 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  gap ,scheduleName, splitValue, payeeName);
> 
> Allan Anderson wrote:
> I think this code was transferred from 
> kmymoney/dialogs/transactionmatcher.cpp lines 79-88, and now, here, I think 
> it deals with matching a schedule.  It's a while since I was in this area, so 
> this could be totally wrong, but I think that in the original position, it 
> also handled matched non-scheduled and non-imported transactions.  Have you 
> tested this capability?  If so, great and I'm wrong.
> 
> Artur Puzio wrote:
> Allan, you are correct, I will check if the `match` function is invoked 
> elsewhere.
> 
> Christian David wrote:
> Actually, I think you are right Allan. But which non-imported 
> transactions are matched? Do you mean user entered transactions in the 
> register?
> 
> Allan Anderson wrote:
> See https://bugs.kde.org/show_bug.cgi?id=333949.
> This added the ability to match two imported transactions, and also two 
> manually entered ones.  Originally, only a combination of one of each could 
> be matched.  No scheduled transactions were changed, although I did add 
> allowing the user to accept a match outside the default date scope.
> 
> Christian David wrote:
> I rechecked this. According to KDevelop’s code parser 
> ```TransactionMatcher::match(...)``` is used three times, namely in:
> 
> MyMoneyStatementReader::handleMatchingOfExistingTransaction(...)
> MyMoneyStatementReader::handleMatchingOfScheduledTransaction(...)
> KMyMoneyApp::transactionMatch()
> 
> The second one is using 
> ```MyMoneyStatementReader::askUserToEnterScheduleForMatching``` where the 
> question was moved to. So the question which is left: do we need the time 
> check in the other two functions (I do not know when they are used, yet).
> 
> However, the question must be moved out of 
> ```TransactionMatcher::match()```. Otherwise the user is asked multiple times 
> if he wants to match a single transaction – a usability sin.
> 
> Btw: ```TransactionMatcher::match()``` should not contain any user 
> interaction by design to keep our codebase readable. It is a backend function.
> 
> Christian David wrote:
> I did some research how the other functions use 
> ```TransactionMatcher::match()```.
> 
> ```KMyMoneyApp::transactionMatch()``` is only called if the user manually 
> starts a match using a button. So the check is reasonable but not necessary – 
> the user probably knows what he is doin

Re: Review Request 126875: Combined the questions from TransactionMatcher::match and MyMoneyStatementReader::askUserToEnterScheduleForMatching

[Allan Anderson]

> On Jan. 24, 2016, 9:02 p.m., Christian David wrote:
> > kmymoney/converter/mymoneystatementreader.cpp, line 1458
> > <https://git.reviewboard.kde.org/r/126875/diff/1/?file=439594#file439594line1458>
> >
> > Translated strings should not be combined, as it confuses the 
> > translators. It is better to create two long strings using i18np() although 
> > this duplicates the text (which is usually bad).
> 
> Artur Puzio wrote:
> But the i18np() is for plurals. What we can do is change the previous 
> string only slitly to add the variable that will contain `"The transaction 
> dates are one day apart."` or `""`.
> 
> Christian David wrote:
> You could use something like
> 
> i18n("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %1"
>  "Transaction: %2 %3"
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  scheduleName, splitValue, payeeName);
>   
> and
> i18np("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>  "The transaction dates are one day 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  "KMyMoney has found a scheduled 
> transaction which matches an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>      "The transaction dates are %1 days 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  gap ,scheduleName, splitValue, payeeName);
> 
> Allan Anderson wrote:
> I think this code was transferred from 
> kmymoney/dialogs/transactionmatcher.cpp lines 79-88, and now, here, I think 
> it deals with matching a schedule.  It's a while since I was in this area, so 
> this could be totally wrong, but I think that in the original position, it 
> also handled matched non-scheduled and non-imported transactions.  Have you 
> tested this capability?  If so, great and I'm wrong.
> 
> Artur Puzio wrote:
> Allan, you are correct, I will check if the `match` function is invoked 
> elsewhere.
> 
> Christian David wrote:
> Actually, I think you are right Allan. But which non-imported 
> transactions are matched? Do you mean user entered transactions in the 
> register?
> 
> Allan Anderson wrote:
> See https://bugs.kde.org/show_bug.cgi?id=333949.
> This added the ability to match two imported transactions, and also two 
> manually entered ones.  Originally, only a combination of one of each could 
> be matched.  No scheduled transactions were changed, although I did add 
> allowing the user to accept a match outside the default date scope.
> 
> Christian David wrote:
> I rechecked this. According to KDevelop’s code parser 
> ```TransactionMatcher::match(...)``` is used three times, namely in:
> 
> MyMoneyStatementReader::handleMatchingOfExistingTransaction(...)
> MyMoneyStatementReader::handleMatchingOfScheduledTransaction(...)
> KMyMoneyApp::transactionMatch()
> 
> The second one is using 
> ```MyMoneyStatementReader::askUserToEnterScheduleForMatching``` where the 
> question was moved to. So the question which is left: do we need the time 
> check in the other two functions (I do not know when they are used, yet).
> 
> However, the question must be moved out of 
> ```TransactionMatcher::match()```. Otherwise the user is asked multiple times 
> if he wants to match a single transaction – a usability sin.
> 
> Btw: ```TransactionMatcher::match()``` should not contain any user 
> interaction by design to keep our codebase readable. It is a backend function.
> 
> Christian David wrote:
> I did some research how the other functions use 
> ```TransactionMatcher::match()```.
> 
> ```KMyMoneyApp::transactionMatch()``` is only called if the user manually 
> starts a match using a button. So the check is reasonable but not necessary – 
> the user probably knows what he is doin

Re: Review Request 126875: Combined the questions from TransactionMatcher::match and MyMoneyStatementReader::askUserToEnterScheduleForMatching

[Allan Anderson]

> On Jan. 24, 2016, 9:02 p.m., Christian David wrote:
> > kmymoney/converter/mymoneystatementreader.cpp, line 1458
> > <https://git.reviewboard.kde.org/r/126875/diff/1/?file=439594#file439594line1458>
> >
> > Translated strings should not be combined, as it confuses the 
> > translators. It is better to create two long strings using i18np() although 
> > this duplicates the text (which is usually bad).
> 
> Artur Puzio wrote:
> But the i18np() is for plurals. What we can do is change the previous 
> string only slitly to add the variable that will contain `"The transaction 
> dates are one day apart."` or `""`.
> 
> Christian David wrote:
> You could use something like
> 
> i18n("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %1"
>  "Transaction: %2 %3"
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  scheduleName, splitValue, payeeName);
>   
> and
> i18np("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>  "The transaction dates are one day 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  "KMyMoney has found a scheduled 
> transaction which matches an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>      "The transaction dates are %1 days 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  gap ,scheduleName, splitValue, payeeName);
> 
> Allan Anderson wrote:
> I think this code was transferred from 
> kmymoney/dialogs/transactionmatcher.cpp lines 79-88, and now, here, I think 
> it deals with matching a schedule.  It's a while since I was in this area, so 
> this could be totally wrong, but I think that in the original position, it 
> also handled matched non-scheduled and non-imported transactions.  Have you 
> tested this capability?  If so, great and I'm wrong.
> 
> Artur Puzio wrote:
> Allan, you are correct, I will check if the `match` function is invoked 
> elsewhere.
> 
> Christian David wrote:
> Actually, I think you are right Allan. But which non-imported 
> transactions are matched? Do you mean user entered transactions in the 
> register?
> 
> Allan Anderson wrote:
> See https://bugs.kde.org/show_bug.cgi?id=333949.
> This added the ability to match two imported transactions, and also two 
> manually entered ones.  Originally, only a combination of one of each could 
> be matched.  No scheduled transactions were changed, although I did add 
> allowing the user to accept a match outside the default date scope.
> 
> Christian David wrote:
> I rechecked this. According to KDevelop’s code parser 
> ```TransactionMatcher::match(...)``` is used three times, namely in:
> 
> MyMoneyStatementReader::handleMatchingOfExistingTransaction(...)
> MyMoneyStatementReader::handleMatchingOfScheduledTransaction(...)
> KMyMoneyApp::transactionMatch()
> 
> The second one is using 
> ```MyMoneyStatementReader::askUserToEnterScheduleForMatching``` where the 
> question was moved to. So the question which is left: do we need the time 
> check in the other two functions (I do not know when they are used, yet).
> 
> However, the question must be moved out of 
> ```TransactionMatcher::match()```. Otherwise the user is asked multiple times 
> if he wants to match a single transaction – a usability sin.
> 
> Btw: ```TransactionMatcher::match()``` should not contain any user 
> interaction by design to keep our codebase readable. It is a backend function.
> 
> Christian David wrote:
> I did some research how the other functions use 
> ```TransactionMatcher::match()```.
> 
> ```KMyMoneyApp::transactionMatch()``` is only called if the user manually 
> starts a match using a button. So the check is reasonable but not necessary – 
> the user probably knows what he is doin

Re: Review Request 126875: Combined the questions from TransactionMatcher::match and MyMoneyStatementReader::askUserToEnterScheduleForMatching

[Allan Anderson]

> On Jan. 24, 2016, 9:02 p.m., Christian David wrote:
> > kmymoney/converter/mymoneystatementreader.cpp, line 1458
> > <https://git.reviewboard.kde.org/r/126875/diff/1/?file=439594#file439594line1458>
> >
> > Translated strings should not be combined, as it confuses the 
> > translators. It is better to create two long strings using i18np() although 
> > this duplicates the text (which is usually bad).
> 
> Artur Puzio wrote:
> But the i18np() is for plurals. What we can do is change the previous 
> string only slitly to add the variable that will contain `"The transaction 
> dates are one day apart."` or `""`.
> 
> Christian David wrote:
> You could use something like
> 
> i18n("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %1"
>  "Transaction: %2 %3"
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  scheduleName, splitValue, payeeName);
>   
> and
> i18np("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>  "The transaction dates are one day 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  "KMyMoney has found a scheduled 
> transaction which matches an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>      "The transaction dates are %1 days 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  gap ,scheduleName, splitValue, payeeName);
> 
> Allan Anderson wrote:
> I think this code was transferred from 
> kmymoney/dialogs/transactionmatcher.cpp lines 79-88, and now, here, I think 
> it deals with matching a schedule.  It's a while since I was in this area, so 
> this could be totally wrong, but I think that in the original position, it 
> also handled matched non-scheduled and non-imported transactions.  Have you 
> tested this capability?  If so, great and I'm wrong.
> 
> Artur Puzio wrote:
> Allan, you are correct, I will check if the `match` function is invoked 
> elsewhere.
> 
> Christian David wrote:
> Actually, I think you are right Allan. But which non-imported 
> transactions are matched? Do you mean user entered transactions in the 
> register?

See https://bugs.kde.org/show_bug.cgi?id=333949.
This added the ability to match two imported transactions, and also two 
manually entered ones.  Originally, only a combination of one of each could be 
matched.  No scheduled transactions were changed, although I did add allowing 
the user to accept a match outside the default date scope.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126875/#review91536
---


On Jan. 24, 2016, 10:07 p.m., Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126875/
> ---
> 
> (Updated Jan. 24, 2016, 10:07 p.m.)
> 
> 
> Review request for KMymoney and Christian David.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Moved the question from `TransactionMatcher::match` to 
> `MyMoneyStatementReader::askUserToEnterScheduleForMatching`.
> Added `MyMoneyTransaction & importedTransaction` parmeter to 
> `MyMoneyStatementReader::askUserToEnterScheduleForMatching`, as 
> `importedTransation` is required to calculate if the question from 
> `TransactionMatcher::match` should be shown.
> 
> 
> Diffs
> -
> 
>   kmymoney/converter/mymoneystatementreader.h 80da202 
>   kmymoney/converter/mymoneystatementreader.cpp 1634bbb 
>   kmymoney/dialogs/transactionmatcher.cpp 7d97404 
> 
> Diff: https://git.reviewboard.kde.org/r/126875/diff/
> 
> 
> Testing
> ---
> 
> Automated tests still pass, but they don't check the subject of work.
> Screenshot: [link](https://imgur.com/3Veh70N)
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

Re: Review Request 126875: Combined the questions from TransactionMatcher::match and MyMoneyStatementReader::askUserToEnterScheduleForMatching

[Allan Anderson]

> On Jan. 24, 2016, 9:02 p.m., Christian David wrote:
> > kmymoney/converter/mymoneystatementreader.cpp, line 1458
> > 
> >
> > Translated strings should not be combined, as it confuses the 
> > translators. It is better to create two long strings using i18np() although 
> > this duplicates the text (which is usually bad).
> 
> Artur Puzio wrote:
> But the i18np() is for plurals. What we can do is change the previous 
> string only slitly to add the variable that will contain `"The transaction 
> dates are one day apart."` or `""`.
> 
> Christian David wrote:
> You could use something like
> 
> i18n("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %1"
>  "Transaction: %2 %3"
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  scheduleName, splitValue, payeeName);
>   
> and
> i18np("KMyMoney has found a scheduled transaction which matches 
> an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>  "The transaction dates are one day 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  "KMyMoney has found a scheduled 
> transaction which matches an imported transaction."
>  "Schedule name: %2"
>  "Transaction: %3 %4"
>  "The transaction dates are %1 days 
> apart."
>  "Do you want KMyMoney to enter this schedule 
> now so that the transaction can be matched?",
>  gap ,scheduleName, splitValue, payeeName);

I think this code was transferred from kmymoney/dialogs/transactionmatcher.cpp 
lines 79-88, and now, here, I think it deals with matching a schedule.  It's a 
while since I was in this area, so this could be totally wrong, but I think 
that in the original position, it also handled matched non-scheduled and 
non-imported transactions.  Have you tested this capability?  If so, great and 
I'm wrong.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126875/#review91536
---


On Jan. 24, 2016, 8:52 p.m., Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126875/
> ---
> 
> (Updated Jan. 24, 2016, 8:52 p.m.)
> 
> 
> Review request for KMymoney and Christian David.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Moved the question from `TransactionMatcher::match` to 
> `MyMoneyStatementReader::askUserToEnterScheduleForMatching`.
> Added `MyMoneyTransaction & importedTransaction` parmeter to 
> `MyMoneyStatementReader::askUserToEnterScheduleForMatching`, as 
> `importedTransation` is required to calculate if the question from 
> `TransactionMatcher::match` should be shown.
> 
> 
> Diffs
> -
> 
>   kmymoney/converter/mymoneystatementreader.h 
> 80da2023490a99963b5ff85c8d77cf1d8b58bf2b 
>   kmymoney/converter/mymoneystatementreader.cpp 
> 1634bbb37ec86d0f065a73427cd353697812c79e 
>   kmymoney/dialogs/transactionmatcher.cpp 
> 7d9740411b96940ac848009218dce65ffaac061e 
> 
> Diff: https://git.reviewboard.kde.org/r/126875/diff/
> 
> 
> Testing
> ---
> 
> Automated tests still pass, but they don't check the subject of work.
> Screenshot: [link](https://imgur.com/3Veh70N)
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

Re: Review Request 124815: BUG:347166 "Price/share" field on investment transaction entry form is mislabeled

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124815/
---

(Updated Jan. 18, 2016, 6:20 p.m.)


Review request for KMymoney.


Changes
---

Where possible, I've removed references to the bug involving editing Dividend 
transactions, as there is now a separate Bug report for this issue. 
(https://bugs.kde.org/show_bug.cgi?id=358129)
This Reviewboard entry has been committed already.  Please ignore second patch.


Bugs: 347166
http://bugs.kde.org/show_bug.cgi?id=347166


Repository: kmymoney


Description (updated)
---

Fix for "Price/share" field on investment transaction entry form is mislabeled.


Diffs
-

  kmymoney/dialogs/investactivities.h d2e2a76 
  kmymoney/dialogs/investactivities.cpp 29c7957 
  kmymoney/dialogs/investtransactioneditor.cpp 882e5a0 

Diff: https://git.reviewboard.kde.org/r/124815/diff/


Testing
---

Create and edit numerous relevant transactions.


Thanks,

Allan Anderson

Re: [Kmymoney-devel] Review Request 124115: BUG:349027 Fix transaction classed as a transfer instead of a withdrawal.

[Allan Anderson]

 On June 17, 2015, 2 p.m., Thomas Baumgart wrote:
  I doubt that this is the solution. I still don't see the problem: 
  withdrawal and deposit are disabled (greyed out) only if the category 
  contains another asset or liability account. it it's empty, all three are 
  accessible. How can I reproduce the problem? Can you attach necessary files 
  to the bug entry?
  
  In case it's a KMyMoney general problem and not related to any import 
  action, one should be capable of entering a transaction via the form to 
  show the problem.
 
 Allan Anderson wrote:
 As I indicated to the OP on the BKO, I don't see, and have never seen, 
 the withdrawal and deposit fields being disabled. He is on, I think, 4.7.1.
 
 So, I concentrated on the mis-classification of a transfer as a 
 withdrawal.
 
 Entering manually a new transaction, or editing an existing withdrawal or 
 deposit, with no category, it will show as a transfer. Open it for editing, 
 and it will then switch to either withdrawal or deposit.  Close without any 
 change, and it reverts to a transfer.  As this seemed similar to the OP's 
 other issue, I investigated that.
 
 Similarly, an imported transaction with no category will show as a 
 transfer, and one with a category will show as either withdrawal or deposit.
 
 Allan Anderson wrote:
 Do I need to be doing anything with this?  I'm thinking of the upcoming 
 4.8.
 
 Thomas Baumgart wrote:
 I investigated this a bit further. The problem can only exist, if the 
 transaction is not categorized. In that case, the transaction has only a 
 single split. In all other cases, the existing logic works as designed and 
 should not be changed.
 
 The fix should keep the current logic unchanged (it will change it in 
 case of a mixed multi-split transaction and shows different results depending 
 on the order of the splits referencing income/expense or asset/liability 
 accounts). If the first one found is income/expense it shows 'Withdrawal' or 
 'Deposit' if it is asset/liability it will show 'Transfer'. The current 
 implementation shows 'Deposit/Withdrawal' for any mixed multi-split 
 transaction.
 
 I suggest to enclose the current logic to cover the corner case of a 
 single split transaction and determine withdrawal/deposit solely on the 
 amount of the split in this case, e.g.
 
 
 KMyMoneyRegister::Action StdTransaction::actionType() const
 {
   KMyMoneyRegister::Action action = ActionNone;
 
   if(m_transaction.splitCount()  1) {
   
 // keep the current logic as is
 
   } else {
 action = m_split.shares().isNegative() ? ActionWithdrawal : 
 ActionDeposit;
   }
   return action;
 }

I'm missing something here.

Without your proposed patch, I still don't see the withdrawal and deposit 
fields being disabled.

If I import a simple QIF file, like -
!Type:Bank
D01-06-08
PADDED NET INT
T80.07
^

initially the Transfer tab is enabled, although clicking on any of them moves 
focus to the following empty transaction. If I click Edit, any tab may be 
selected.  If I select Withdrawal, and edit the payee and add a category, as 
per the OP, both the Deposit and Withdrawal tabs are enabled.  Clicking Enter, 
the transaction is saved correctly.  If I don't add a category, all three are 
still enabled.  If I enter an asset or liability account, only then are they 
disabled.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124115/#review81525
---


On Aug. 19, 2015, 10:15 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124115/
 ---
 
 (Updated Aug. 19, 2015, 10:15 a.m.)
 
 
 Review request for KMymoney and Thomas Baumgart.
 
 
 Bugs: 349027
 http://bugs.kde.org/show_bug.cgi?id=349027
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Initially reported as problem with QIF file import incorrectly classing a 
 transaction as a transfer instead of a withdrawal, but also found in CSV 
 importing.  In fact, it isn't really an importing problem, but in KMyMoney 
 itself.
 
 
 Diffs
 -
 
   kmymoney/widgets/transaction.cpp 77bbcb1 
 
 Diff: https://git.reviewboard.kde.org/r/124115/diff/
 
 
 Testing
 ---
 
 Checked with downloads and also manually edited transactions.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 124815: BUG:347166 Price/share field on investment transaction entry form is mislabeled

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124815/
---

(Updated Aug. 20, 2015, 10:35 a.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Repository: kmymoney


Description
---

Fix for Price/share field on investment transaction entry form is mislabeled.


Diffs
-

  kmymoney/dialogs/investactivities.h d2e2a76 
  kmymoney/dialogs/investactivities.cpp 29c7957 
  kmymoney/dialogs/investtransactioneditor.cpp 882e5a0 

Diff: https://git.reviewboard.kde.org/r/124815/diff/


Testing
---

Create and edit numerous relevant transactions.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 124815: BUG:347166 Price/share field on investment transaction entry form is mislabeled

[Allan Anderson]

 On Aug. 19, 2015, 11:02 a.m., Thomas Baumgart wrote:
 

Thanks, Thomas.  I know you're busy.
I've attended to both those points.
Do you want to see the revised patch, or am I OK to push?


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124815/#review84041
---


On Aug. 19, 2015, 10:14 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124815/
 ---
 
 (Updated Aug. 19, 2015, 10:14 a.m.)
 
 
 Review request for KMymoney.
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Fix for Price/share field on investment transaction entry form is 
 mislabeled.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.h d2e2a76 
   kmymoney/dialogs/investactivities.cpp 29c7957 
   kmymoney/dialogs/investtransactioneditor.cpp 882e5a0 
 
 Diff: https://git.reviewboard.kde.org/r/124815/diff/
 
 
 Testing
 ---
 
 Create and edit numerous relevant transactions.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 124815: BUG:347166 Price/share field on investment transaction entry form is mislabeled

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124815/
---

(Updated Aug. 19, 2015, 10:14 a.m.)


Review request for KMymoney.


Changes
---

BUG:347166 Fix Price/share field on investment transaction entry form is 
mislabeled.


Repository: kmymoney


Description (updated)
---

Fix for Price/share field on investment transaction entry form is mislabeled.


Diffs
-

  kmymoney/dialogs/investactivities.h d2e2a76 
  kmymoney/dialogs/investactivities.cpp 29c7957 
  kmymoney/dialogs/investtransactioneditor.cpp 882e5a0 

Diff: https://git.reviewboard.kde.org/r/124815/diff/


Testing
---

Create and edit numerous relevant transactions.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 124815: BUG:347166 Price/share field on investment transaction entry form is mislabeled

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124815/
---

Review request for KMymoney.


Repository: kmymoney


Description
---

Fix for Price/share field on investment transaction entry form is mislabeled


Diffs
-

  kmymoney/dialogs/investactivities.h d2e2a76 
  kmymoney/dialogs/investactivities.cpp 29c7957 
  kmymoney/dialogs/investtransactioneditor.cpp 882e5a0 

Diff: https://git.reviewboard.kde.org/r/124815/diff/


Testing
---

Create and edit numerous relevant transactions.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 124115: BUG:349027 Fix transaction classed as a transfer instead of a withdrawal.

[Allan Anderson]

 On June 17, 2015, 2 p.m., Thomas Baumgart wrote:
  I doubt that this is the solution. I still don't see the problem: 
  withdrawal and deposit are disabled (greyed out) only if the category 
  contains another asset or liability account. it it's empty, all three are 
  accessible. How can I reproduce the problem? Can you attach necessary files 
  to the bug entry?
  
  In case it's a KMyMoney general problem and not related to any import 
  action, one should be capable of entering a transaction via the form to 
  show the problem.
 
 Allan Anderson wrote:
 As I indicated to the OP on the BKO, I don't see, and have never seen, 
 the withdrawal and deposit fields being disabled. He is on, I think, 4.7.1.
 
 So, I concentrated on the mis-classification of a transfer as a 
 withdrawal.
 
 Entering manually a new transaction, or editing an existing withdrawal or 
 deposit, with no category, it will show as a transfer. Open it for editing, 
 and it will then switch to either withdrawal or deposit.  Close without any 
 change, and it reverts to a transfer.  As this seemed similar to the OP's 
 other issue, I investigated that.
 
 Similarly, an imported transaction with no category will show as a 
 transfer, and one with a category will show as either withdrawal or deposit.

Do I need to be doing anything with this?  I'm thinking of the upcoming 4.8.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124115/#review81525
---


On June 17, 2015, 1:11 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124115/
 ---
 
 (Updated June 17, 2015, 1:11 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 349027
 http://bugs.kde.org/show_bug.cgi?id=349027
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Initially reported as problem with QIF file import incorrectly classing a 
 transaction as a transfer instead of a withdrawal, but also found in CSV 
 importing.  In fact, it isn't really an importing problem, but in KMyMoney 
 itself.
 
 
 Diffs
 -
 
   kmymoney/widgets/transaction.cpp 77bbcb1 
 
 Diff: https://git.reviewboard.kde.org/r/124115/diff/
 
 
 Testing
 ---
 
 Checked with downloads and also manually edited transactions.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 124115: BUG:349027 Fix transaction classed as a transfer instead of a withdrawal.

[Allan Anderson]

 On June 17, 2015, 2 p.m., Thomas Baumgart wrote:
  I doubt that this is the solution. I still don't see the problem: 
  withdrawal and deposit are disabled (greyed out) only if the category 
  contains another asset or liability account. it it's empty, all three are 
  accessible. How can I reproduce the problem? Can you attach necessary files 
  to the bug entry?
  
  In case it's a KMyMoney general problem and not related to any import 
  action, one should be capable of entering a transaction via the form to 
  show the problem.

As I indicated to the OP on the BKO, I don't see, and have never seen, the 
withdrawal and deposit fields being disabled. He is on, I think, 4.7.1.

So, I concentrated on the mis-classification of a transfer as a withdrawal.

Entering manually a new transaction, or editing an existing withdrawal or 
deposit, with no category, it will show as a transfer. Open it for editing, and 
it will then switch to either withdrawal or deposit.  Close without any change, 
and it reverts to a transfer.  As this seemed similar to the OP's other issue, 
I investigated that.

Similarly, an imported transaction with no category will show as a transfer, 
and one with a category will show as either withdrawal or deposit.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124115/#review81525
---


On June 17, 2015, 1:11 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124115/
 ---
 
 (Updated June 17, 2015, 1:11 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 349027
 http://bugs.kde.org/show_bug.cgi?id=349027
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Initially reported as problem with QIF file import incorrectly classing a 
 transaction as a transfer instead of a withdrawal, but also found in CSV 
 importing.  In fact, it isn't really an importing problem, but in KMyMoney 
 itself.
 
 
 Diffs
 -
 
   kmymoney/widgets/transaction.cpp 77bbcb1 
 
 Diff: https://git.reviewboard.kde.org/r/124115/diff/
 
 
 Testing
 ---
 
 Checked with downloads and also manually edited transactions.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 124115: BUG:349027 Fix transaction classed as a transfer instead of a withdrawal.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124115/
---

Review request for KMymoney.


Bugs: 349027
http://bugs.kde.org/show_bug.cgi?id=349027


Repository: kmymoney


Description
---

Initially reported as problem with QIF file import incorrectly classing a 
transaction as a transfer instead of a withdrawal, but also found in CSV 
importing.  In fact, it isn't really an importing problem, but in KMyMoney 
itself.


Diffs
-

  kmymoney/widgets/transaction.cpp 77bbcb1 

Diff: https://git.reviewboard.kde.org/r/124115/diff/


Testing
---

Checked with downloads and also manually edited transactions.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122364/
---

(Updated March 28, 2015, 3:06 p.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Changes
---

Submitted with commit 0dc7e903d389c458289a92634f9e346928316173 by Allan 
Anderson to branch master.


Bugs: 340656
http://bugs.kde.org/show_bug.cgi?id=340656


Repository: kmymoney


Description
---

Fix display on high-definition monitors. Fix interaction between the import 
preview table widget and the parameter entry wizard, caused by them both 
resizing dynamically within the same window. This was achieved by creating a 
new class and UI for the wizard and transferring most of the existing relevant 
code out of the large csvdialog.cpp file into the new class and file.  
Unfortunately, because 11 UIs are affected and the transfer of existing logic 
into a new class, the patch is quite large, although there is little new code 
involved.


Diffs
-

  kmymoney/plugins/csvimport/csvwizard.h PRE-CREATION 
  kmymoney/plugins/csvimport/csvwizard.cpp PRE-CREATION 
  kmymoney/plugins/csvimport/csvwizard.ui PRE-CREATION 
  kmymoney/plugins/csvimport/introwizardpage.ui 2bf93f0 
  kmymoney/plugins/csvimport/investmentdlg.cpp 5e7ce06 
  kmymoney/plugins/csvimport/investmentwizardpage.ui d3f2857 
  kmymoney/plugins/csvimport/investprocessing.h 3c08dee 
  kmymoney/plugins/csvimport/investprocessing.cpp 2b6b2d1 
  kmymoney/plugins/csvimport/lines-datewizardpage.ui 01d7253 
  kmymoney/plugins/csvimport/redefinedlgdecl.ui 26d8b62 
  kmymoney/plugins/csvimport/separatorwizardpage.ui 21136f2 
  kmymoney/plugins/csvimport/CMakeLists.txt 36e5afc 
  kmymoney/plugins/csvimport/bankingwizardpage.ui 9e1b5cb 
  kmymoney/plugins/csvimport/completionwizardpage.ui ce61e89 
  kmymoney/plugins/csvimport/csvdialog.h 780329d 
  kmymoney/plugins/csvimport/csvdialog.cpp b986317 
  kmymoney/plugins/csvimport/csvdialog.ui 166b04a 

Diff: https://git.reviewboard.kde.org/r/122364/diff/


Testing
---

Tested on 96 DPI and 160 DPI.  Also, the OP of the bug has tested the patch and 
confirms its performance.
There is one remaining UI not yet included.  I've completed its testing, but as 
it's a little-used window, I've held it back to avoid making this patch even 
larger.
I'm unable to test on Windows.


File Attachments


Updated patch
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/03/6e4dad98-936c-4ca0-8804-c4abb9b51438__0002-BUG-340656.patch


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122364/#review76808
---


Might someone be free to OK this, or otherwise, please.  In spite of its size, 
there is not a huge amount of change involved.

- Allan Anderson


On Feb. 3, 2015, 12:27 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122364/
 ---
 
 (Updated Feb. 3, 2015, 12:27 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 340656
 http://bugs.kde.org/show_bug.cgi?id=340656
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Fix display on high-definition monitors. Fix interaction between the import 
 preview table widget and the parameter entry wizard, caused by them both 
 resizing dynamically within the same window. This was achieved by creating a 
 new class and UI for the wizard and transferring most of the existing 
 relevant code out of the large csvdialog.cpp file into the new class and 
 file.  Unfortunately, because 11 UIs are affected and the transfer of 
 existing logic into a new class, the patch is quite large, although there is 
 little new code involved.
 
 
 Diffs
 -
 
   kmymoney/plugins/csvimport/csvwizard.h PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.cpp PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.ui PRE-CREATION 
   kmymoney/plugins/csvimport/introwizardpage.ui 2bf93f0 
   kmymoney/plugins/csvimport/investmentdlg.cpp 5e7ce06 
   kmymoney/plugins/csvimport/investmentwizardpage.ui d3f2857 
   kmymoney/plugins/csvimport/investprocessing.h 3c08dee 
   kmymoney/plugins/csvimport/investprocessing.cpp 2b6b2d1 
   kmymoney/plugins/csvimport/lines-datewizardpage.ui 01d7253 
   kmymoney/plugins/csvimport/redefinedlgdecl.ui 26d8b62 
   kmymoney/plugins/csvimport/separatorwizardpage.ui 21136f2 
   kmymoney/plugins/csvimport/CMakeLists.txt 36e5afc 
   kmymoney/plugins/csvimport/bankingwizardpage.ui 9e1b5cb 
   kmymoney/plugins/csvimport/completionwizardpage.ui ce61e89 
   kmymoney/plugins/csvimport/csvdialog.h 780329d 
   kmymoney/plugins/csvimport/csvdialog.cpp b986317 
   kmymoney/plugins/csvimport/csvdialog.ui 166b04a 
 
 Diff: https://git.reviewboard.kde.org/r/122364/diff/
 
 
 Testing
 ---
 
 Tested on 96 DPI and 160 DPI.  Also, the OP of the bug has tested the patch 
 and confirms its performance.
 There is one remaining UI not yet included.  I've completed its testing, but 
 as it's a little-used window, I've held it back to avoid making this patch 
 even larger.
 I'm unable to test on Windows.
 
 
 File Attachments
 
 
 Updated patch
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/02/03/6e4dad98-936c-4ca0-8804-c4abb9b51438__0002-BUG-340656.patch
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment365
 
  ```Qt::WindowFlags eFlags = windowFlags();``` (space removed)
 
 Allan Anderson wrote:
 I know it looks odd, but I copied this from a google search.  If I 
 remember, the point was to avoid the window in question staying on top.
 Perhaps I misunderstood?  I didn't actually try without it.

see below


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment366
 
  This part can be removed, the flag is set and removed again. Also you 
  call show() twice.
 
 Allan Anderson wrote:
 As above.
 
 Christian David wrote:
 Without seeing the code: I think, you should just drop the part where the 
 flag is removed again and the second ```show()```.
 
 Allan Anderson wrote:
 This is used in three places, and in two of them problems ensue.  In the 
 first one, the wizard window stays on top and clicking on the table window 
 does not bring it to the front.  It is necessary to start to manipulate the 
 wizard before the table wakes up.  In the second one, it is possible for the 
 table to completely cover the wizard, and to circumvent this, I allow a ight 
 click on the table to raise the wizard.  With those three lines removed, the 
 wizard stays hidden.  In the third case, it might be possible to remove all 
 that code, with no problem apparent.  I'll keep my eye on all this.

No other issues.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122364/#review75942
---


On Feb. 3, 2015, 12:27 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122364/
 ---
 
 (Updated Feb. 3, 2015, 12:27 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 340656
 http://bugs.kde.org/show_bug.cgi?id=340656
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Fix display on high-definition monitors. Fix interaction between the import 
 preview table widget and the parameter entry wizard, caused by them both 
 resizing dynamically within the same window. This was achieved by creating a 
 new class and UI for the wizard and transferring most of the existing 
 relevant code out of the large csvdialog.cpp file into the new class and 
 file.  Unfortunately, because 11 UIs are affected and the transfer of 
 existing logic into a new class, the patch is quite large, although there is 
 little new code involved.
 
 
 Diffs
 -
 
   kmymoney/plugins/csvimport/csvwizard.h PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.cpp PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.ui PRE-CREATION 
   kmymoney/plugins/csvimport/introwizardpage.ui 2bf93f0 
   kmymoney/plugins/csvimport/investmentdlg.cpp 5e7ce06 
   kmymoney/plugins/csvimport/investmentwizardpage.ui d3f2857 
   kmymoney/plugins/csvimport/investprocessing.h 3c08dee 
   kmymoney/plugins/csvimport/investprocessing.cpp 2b6b2d1 
   kmymoney/plugins/csvimport/lines-datewizardpage.ui 01d7253 
   kmymoney/plugins/csvimport/redefinedlgdecl.ui 26d8b62 
   kmymoney/plugins/csvimport/separatorwizardpage.ui 21136f2 
   kmymoney/plugins/csvimport/CMakeLists.txt 36e5afc 
   kmymoney/plugins/csvimport/bankingwizardpage.ui 9e1b5cb 
   kmymoney/plugins/csvimport/completionwizardpage.ui ce61e89 
   kmymoney/plugins/csvimport/csvdialog.h 780329d 
   kmymoney/plugins/csvimport/csvdialog.cpp b986317 
   kmymoney/plugins/csvimport/csvdialog.ui 166b04a 
 
 Diff: https://git.reviewboard.kde.org/r/122364/diff/
 
 
 Testing
 ---
 
 Tested on 96 DPI and 160 DPI.  Also, the OP of the bug has tested the patch 
 and confirms its performance.
 There is one remaining UI not yet included.  I've completed its testing, but 
 as it's a little-used window, I've held it back to avoid making this patch 
 even larger.
 I'm unable to test on Windows.
 
 
 File Attachments
 
 
 Updated patch
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/02/03/6e4dad98-936c-4ca0-8804-c4abb9b51438__0002-BUG-340656.patch
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment357
 
  This should be done in a more modular fashion (general annotation).
 
 Allan Anderson wrote:
 Sorry, can you explain.
 
 Christian David wrote:
 Imagine you want to change or work with the object pointed to by 
 ```m_pageCompletion```. Then you have to look at ```m_wiz``` as well – from 
 experience I know this is forgotten easily. Also it makes it hard to 
 understand the code. The chance a new developer will give up after a couple 
 of these constructs is high.
 
 This line is in a method called ```init()```. So I assume it is called 
 once to initialize the widget. So I would move the initializing into the 
 class of ```m_pageCompletion```. Actually you can set this simply in Qt 
 Designer and do not need any C++ code.

m_pageCompletion is the final page of the importer wizard.  Each of the six 
pages has its own ui and has to be accessed via the wizard itself, which is 
m_wiz.  I'm not aware of any mechanism that allows a wizard ui component to be 
directly accessed.
Then, in this most recent incarnation of the importer, the wizard and its 
various uis were split off into a separate class, from the main importer which 
handles the preview QWidgetTable.  The preview table is used by both the 
banking side and the investment side, both of which display the data in the 
table, according to the fields selected by the user.  So, both of them need to 
access the wizard pages, each having a pointer to the wizard, which in turn 
manipulates the various pages.

This function is not used just at startup, but also during program exection.  
If you move the initializing of the combobox into the class of 
m_pageCompletion, the decision/need to clear it or reset it still has to come 
from the banking or investment class.  So I don't think it would achieve 
anything.

As ever, I'm willing to learn.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment358
 
  This part can be removed, the flag is set and removed again. Also you 
  call show() twice.
 
 Allan Anderson wrote:
 As above.
 
 Christian David wrote:
 Without seeing the code: I think, you should just drop the part where the 
 flag is removed again and the second ```show()```.

This is used in three places, and in two of them problems ensue.  In the first 
one, the wizard window stays on top and clicking on the table window does not 
bring it to the front.  It is necessary to start to manipulate the wizard 
before the table wakes up.  In the second one, it is possible for the table to 
completely cover the wizard, and to circumvent this, I allow a ight click on 
the table to raise the wizard.  With those three lines removed, the wizard 
stays hidden.  In the third case, it might be possible to remove all that code, 
with no problem apparent.  I'll keep my eye on all this.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment359
 
  You are calling setCurrentIndex() later again. So these lines are not 
  needed as their change is overwritten anyway (or do you need the change 
  signal to get emitted?).
 
 Allan Anderson wrote:
 If the second setCurrentIndex() doesn't change the setting, there will be 
 no connect, which is needed to allow other settings
 to be checked for conflict, etc..  So, yes, the first is to ensure the 
 second does cause a connect.
 
 Christian David wrote:
 Do you mean a second connect or ensure it causes an emit of the 
 signal?

Perhaps there's a fine point I'm missing here.  The second setCurrentIndex() 
will cause an emit, provided the value has changed.  The first 
setCurrentIndex() clears the setting, and ensures that the second one will get 
triggered.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment360
 
  Why do you need the ui to change?
 
 Allan Anderson wrote:
 As above, really.

The program detects the change and starts a validation process.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment361
 
  Why do you disconnect and reconnect later? Also this connection should 
  be done in CSVWizard.
 
 Allan Anderson wrote:
 This goes back to when I was originally working on the plugins, and if I 
 remember, just above these re-connects, I populate all the comboboxes with 
 their items, which was causing multiple connects and upsetting the indexes.  
 There is a brief note there.
 Agreed about moving them, but do we want more code at the moment

Re: [Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

 hadn't come across that.  If I've used it, it's by accident.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment340
 
  I am not sure here; do these values depend on a user selection? If so 
  it is usualy better to calculate them on request. Having a copy quickly 
  leads to inconsistent data.

Yes, for the Banking and Investment wizards, etc, a group of similar, required 
settings are tested to ensure all required (and optional alternatives) fields 
have been selected.  So, you're saying not to keep these results, but to 
perform the tests as needed?  I was using them as a cache, to reduce the 
library overheads.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment341
 
  Library includes should be above project includes.

I think sometimes I've done that, without knowing it was a requirement/style 
issue.  What about the header #include in a cpp file?


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment342
 
  else if

Of course.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment343
 
  else if

etc. for the others.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment344
 
  This method looks very similar to the following ```…ColumnSelected()``` 
  methods. They could be compacted with some template magic.

Hmmm. I'll have a play with that later.


 On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
  File Attachment: Updated patch - 0002-BUG-340656.patch
  https://git.reviewboard.kde.org/r/122364/#fcomment345
 
  Can this ```center``` work? There is no new paragraph or line break.

Yes, it does work the way I expected.  The second sentence is centered.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122364/#review75942
---


On Feb. 3, 2015, 12:27 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122364/
 ---
 
 (Updated Feb. 3, 2015, 12:27 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 340656
 http://bugs.kde.org/show_bug.cgi?id=340656
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Fix display on high-definition monitors. Fix interaction between the import 
 preview table widget and the parameter entry wizard, caused by them both 
 resizing dynamically within the same window. This was achieved by creating a 
 new class and UI for the wizard and transferring most of the existing 
 relevant code out of the large csvdialog.cpp file into the new class and 
 file.  Unfortunately, because 11 UIs are affected and the transfer of 
 existing logic into a new class, the patch is quite large, although there is 
 little new code involved.
 
 
 Diffs
 -
 
   kmymoney/plugins/csvimport/csvwizard.h PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.cpp PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.ui PRE-CREATION 
   kmymoney/plugins/csvimport/introwizardpage.ui 2bf93f0 
   kmymoney/plugins/csvimport/investmentdlg.cpp 5e7ce06 
   kmymoney/plugins/csvimport/investmentwizardpage.ui d3f2857 
   kmymoney/plugins/csvimport/investprocessing.h 3c08dee 
   kmymoney/plugins/csvimport/investprocessing.cpp 2b6b2d1 
   kmymoney/plugins/csvimport/lines-datewizardpage.ui 01d7253 
   kmymoney/plugins/csvimport/redefinedlgdecl.ui 26d8b62 
   kmymoney/plugins/csvimport/separatorwizardpage.ui 21136f2 
   kmymoney/plugins/csvimport/CMakeLists.txt 36e5afc 
   kmymoney/plugins/csvimport/bankingwizardpage.ui 9e1b5cb 
   kmymoney/plugins/csvimport/completionwizardpage.ui ce61e89 
   kmymoney/plugins/csvimport/csvdialog.h 780329d 
   kmymoney/plugins/csvimport/csvdialog.cpp b986317 
   kmymoney/plugins/csvimport/csvdialog.ui 166b04a 
 
 Diff: https://git.reviewboard.kde.org/r/122364/diff/
 
 
 Testing
 ---
 
 Tested on 96 DPI and 160 DPI.  Also, the OP of the bug has tested the patch 
 and confirms its performance.
 There is one remaining UI not yet included.  I've completed its testing, but 
 as it's a little-used window, I've held it back to avoid making this patch 
 even larger.
 I'm unable to test on Windows.
 
 
 File Attachments
 
 
 Updated patch
   
 https://git.reviewboard.kde.org/media/uploaded/files/2015/02/03/6e4dad98-936c-4ca0-8804

Re: [Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122364/
---

(Updated Feb. 3, 2015, 12:27 a.m.)


Review request for KMymoney.


Changes
---

Replacement patch, in place of original, with conflict fixed, having now done 
the update from Master.  Apologies again.


Bugs: 340656
http://bugs.kde.org/show_bug.cgi?id=340656


Repository: kmymoney


Description
---

Fix display on high-definition monitors. Fix interaction between the import 
preview table widget and the parameter entry wizard, caused by them both 
resizing dynamically within the same window. This was achieved by creating a 
new class and UI for the wizard and transferring most of the existing relevant 
code out of the large csvdialog.cpp file into the new class and file.  
Unfortunately, because 11 UIs are affected and the transfer of existing logic 
into a new class, the patch is quite large, although there is little new code 
involved.


Diffs
-

  kmymoney/plugins/csvimport/csvwizard.h PRE-CREATION 
  kmymoney/plugins/csvimport/csvwizard.cpp PRE-CREATION 
  kmymoney/plugins/csvimport/csvwizard.ui PRE-CREATION 
  kmymoney/plugins/csvimport/introwizardpage.ui 2bf93f0 
  kmymoney/plugins/csvimport/investmentdlg.cpp 5e7ce06 
  kmymoney/plugins/csvimport/investmentwizardpage.ui d3f2857 
  kmymoney/plugins/csvimport/investprocessing.h 3c08dee 
  kmymoney/plugins/csvimport/investprocessing.cpp 2b6b2d1 
  kmymoney/plugins/csvimport/lines-datewizardpage.ui 01d7253 
  kmymoney/plugins/csvimport/redefinedlgdecl.ui 26d8b62 
  kmymoney/plugins/csvimport/separatorwizardpage.ui 21136f2 
  kmymoney/plugins/csvimport/CMakeLists.txt 36e5afc 
  kmymoney/plugins/csvimport/bankingwizardpage.ui 9e1b5cb 
  kmymoney/plugins/csvimport/completionwizardpage.ui ce61e89 
  kmymoney/plugins/csvimport/csvdialog.h 780329d 
  kmymoney/plugins/csvimport/csvdialog.cpp b986317 
  kmymoney/plugins/csvimport/csvdialog.ui 166b04a 

Diff: https://git.reviewboard.kde.org/r/122364/diff/


Testing
---

Tested on 96 DPI and 160 DPI.  Also, the OP of the bug has tested the patch and 
confirms its performance.
There is one remaining UI not yet included.  I've completed its testing, but as 
it's a little-used window, I've held it back to avoid making this patch even 
larger.
I'm unable to test on Windows.


File Attachments (updated)


Updated patch
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/02/03/6e4dad98-936c-4ca0-8804-c4abb9b51438__0002-BUG-340656.patch


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122364/
---

Review request for KMymoney.


Bugs: 340656
http://bugs.kde.org/show_bug.cgi?id=340656


Repository: kmymoney


Description
---

Fix display on high-definition monitors. Fix interaction between the import 
preview table widget and the parameter entry wizard, caused by them both 
resizing dynamically within the same window. This was achieved by creating a 
new class and UI for the wizard and transferring most of the existing relevant 
code out of the large csvdialog.cpp file into the new class and file.  
Unfortunately, because 11 UIs are affected and the transfer of existing logic 
into a new class, the patch is quite large, although there is little new code 
involved.


Diffs
-

  kmymoney/plugins/csvimport/csvwizard.h PRE-CREATION 
  kmymoney/plugins/csvimport/csvwizard.cpp PRE-CREATION 
  kmymoney/plugins/csvimport/csvwizard.ui PRE-CREATION 
  kmymoney/plugins/csvimport/introwizardpage.ui 2bf93f0 
  kmymoney/plugins/csvimport/investmentdlg.cpp 5e7ce06 
  kmymoney/plugins/csvimport/investmentwizardpage.ui d3f2857 
  kmymoney/plugins/csvimport/investprocessing.h 3c08dee 
  kmymoney/plugins/csvimport/investprocessing.cpp 2b6b2d1 
  kmymoney/plugins/csvimport/lines-datewizardpage.ui 01d7253 
  kmymoney/plugins/csvimport/redefinedlgdecl.ui 26d8b62 
  kmymoney/plugins/csvimport/separatorwizardpage.ui 21136f2 
  kmymoney/plugins/csvimport/CMakeLists.txt 36e5afc 
  kmymoney/plugins/csvimport/bankingwizardpage.ui 9e1b5cb 
  kmymoney/plugins/csvimport/completionwizardpage.ui ce61e89 
  kmymoney/plugins/csvimport/csvdialog.h 780329d 
  kmymoney/plugins/csvimport/csvdialog.cpp b986317 
  kmymoney/plugins/csvimport/csvdialog.ui 166b04a 

Diff: https://git.reviewboard.kde.org/r/122364/diff/


Testing
---

Tested on 96 DPI and 160 DPI.  Also, the OP of the bug has tested the patch and 
confirms its performance.
There is one remaining UI not yet included.  I've completed its testing, but as 
it's a little-used window, I've held it back to avoid making this patch even 
larger.
I'm unable to test on Windows.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 122364: CSV Importer - Fix display on high-definition displays. Fix interaction between QTable and QWizard in same window.

[Allan Anderson]

 On Feb. 1, 2015, 5:13 p.m., Christian David wrote:
  I cannot apply you patch because of this error:
  
  error: patch failed: kmymoney/plugins/csvimport/csvdialog.h:501
  error: kmymoney/plugins/csvimport/csvdialog.h: patch does not apply
  error: patch failed: kmymoney/plugins/csvimport/csvdialog.cpp:182
  error: kmymoney/plugins/csvimport/csvdialog.cpp: patch does not apply
  
  Is it outdated? On which commit is it based?

Many apologies, Christian and all.

Stupidly, I omitted to update after my local commit - never done that before.

I noticed fairly quickly, and wanted to warn, but my real life got in the way.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122364/#review75159
---


On Feb. 1, 2015, 2:04 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122364/
 ---
 
 (Updated Feb. 1, 2015, 2:04 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 340656
 http://bugs.kde.org/show_bug.cgi?id=340656
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Fix display on high-definition monitors. Fix interaction between the import 
 preview table widget and the parameter entry wizard, caused by them both 
 resizing dynamically within the same window. This was achieved by creating a 
 new class and UI for the wizard and transferring most of the existing 
 relevant code out of the large csvdialog.cpp file into the new class and 
 file.  Unfortunately, because 11 UIs are affected and the transfer of 
 existing logic into a new class, the patch is quite large, although there is 
 little new code involved.
 
 
 Diffs
 -
 
   kmymoney/plugins/csvimport/csvwizard.h PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.cpp PRE-CREATION 
   kmymoney/plugins/csvimport/csvwizard.ui PRE-CREATION 
   kmymoney/plugins/csvimport/introwizardpage.ui 2bf93f0 
   kmymoney/plugins/csvimport/investmentdlg.cpp 5e7ce06 
   kmymoney/plugins/csvimport/investmentwizardpage.ui d3f2857 
   kmymoney/plugins/csvimport/investprocessing.h 3c08dee 
   kmymoney/plugins/csvimport/investprocessing.cpp 2b6b2d1 
   kmymoney/plugins/csvimport/lines-datewizardpage.ui 01d7253 
   kmymoney/plugins/csvimport/redefinedlgdecl.ui 26d8b62 
   kmymoney/plugins/csvimport/separatorwizardpage.ui 21136f2 
   kmymoney/plugins/csvimport/CMakeLists.txt 36e5afc 
   kmymoney/plugins/csvimport/bankingwizardpage.ui 9e1b5cb 
   kmymoney/plugins/csvimport/completionwizardpage.ui ce61e89 
   kmymoney/plugins/csvimport/csvdialog.h 780329d 
   kmymoney/plugins/csvimport/csvdialog.cpp b986317 
   kmymoney/plugins/csvimport/csvdialog.ui 166b04a 
 
 Diff: https://git.reviewboard.kde.org/r/122364/diff/
 
 
 Testing
 ---
 
 Tested on 96 DPI and 160 DPI.  Also, the OP of the bug has tested the patch 
 and confirms its performance.
 There is one remaining UI not yet included.  I've completed its testing, but 
 as it's a little-used window, I've held it back to avoid making this patch 
 even larger.
 I'm unable to test on Windows.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 120153: Remove the OFX investment transactions amount sign inversion hack.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120153/#review71206
---


I've been drawn back to this because of another bug I'm looking at -
[Bug 322381] Fees(taxes) are deducted twice from dividend amount after QIF 
import.
As mymoneystatementreader.cpp is involve also in QIF and CSV importing, I 
suspect
there may be a side effect of this change.
In mymoneyqifreader.cpp, I see the following -

// For historic reasons (coming from the OFX importer) the statement reader
 // expects the dividend with a reverse sign. So we just do that.
tr.m_amount = -(amount - tr.m_fees);

Will not a corresponding change be required here?  I suspect this may be in part
contributing to Bug 322381.  I've removed the -ve sign and it does partially
correct this problem, but I need to look deeper because I suspect there is an
earlier bug too.

- Allan Anderson


On Sept. 21, 2014, 9:33 a.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120153/
 ---
 
 (Updated Sept. 21, 2014, 9:33 a.m.)
 
 
 Review request for KMymoney and Allan Anderson.
 
 
 Bugs: 333522
 http://bugs.kde.org/show_bug.cgi?id=333522
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 As the author of the hack states he's not sure why this was necessary.
 This hack also caused a lot of workarounds in MyMoneyStatementReader
 so it definitely should be removed. I've tried to keep the current
 behaviour in MyMoneyStatementReader but since I'm not familiar with
 investment transaction these should be double checked.
 
 BUG: 333522
 
 
 Diffs
 -
 
   kmymoney/converter/mymoneystatementreader.cpp 
 766d2151e17f07891f43e6d4d50861d40cbffe17 
   kmymoney/plugins/ofximport/ofximporterplugin.cpp 
 556cb4195ef1c5680ac50ce394f8bd9893fdcbdb 
 
 Diff: https://git.reviewboard.kde.org/r/120153/diff/
 
 
 Testing
 ---
 
 Imported the OFX file attached to BUG 333522 in a checking account and in an 
 investemnt account. Allan please take a look if the investment transactions 
 part in MyMoneyStatementReader is OK.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 120507: CSV Plugin - Improve functionality across distros. Improve UI to display the whole of the file being imported, and smarten to avoid displaying split rows.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120507/
---

(Updated Oct. 16, 2014, 12:15 p.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Repository: kmymoney


Description
---

Improve functionality across distros.
It was also the intention to save the windows settings, but it was realised 
that there was a liklihood that the next file to be imported could have quite 
different column width and number of rows. So, it was decided to  display the 
whole of the file being imported instead.  
Also,the UI has been improved to avoid displaying split rows, with all possible 
rows being displayed, the window height aligning with the rows, and the width 
displaying complete columns, as far as the screen allowed.  Further, I reworked 
some areas of the code to avoid duplication and improve re-usability.


Diffs
-

  kmymoney/plugins/csvimport/lines-datewizardpage.ui 884f3ba 
  kmymoney/plugins/csvimport/separatorwizardpage.ui b6dba9f 
  kmymoney/plugins/csvimport/bankingwizardpage.ui 0d6a4a8 
  kmymoney/plugins/csvimport/csvdialog.h 24abd9a 
  kmymoney/plugins/csvimport/csvdialog.cpp ab20ed4 
  kmymoney/plugins/csvimport/csvdialog.ui 61ab9ce 
  kmymoney/plugins/csvimport/csvimporterplugin.h d2d2f6c 
  kmymoney/plugins/csvimport/csvimporterplugin.cpp 602b4d9 
  kmymoney/plugins/csvimport/introwizardpage.ui a597d56 
  kmymoney/plugins/csvimport/investmentdlg.cpp 5e7b266 
  kmymoney/plugins/csvimport/investmentwizardpage.ui 846836e 
  kmymoney/plugins/csvimport/investprocessing.h fa8ffa0 
  kmymoney/plugins/csvimport/investprocessing.cpp 1629e14 

Diff: https://git.reviewboard.kde.org/r/120507/diff/


Testing
---

Tested with numerous banking and investment files from different sources, on 
Linux Mint KDE and Ubuntu Unity.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 120507: CSV Plugin - Improve functionality across distros. Improve UI to display the whole of the file being imported, and smarten to avoid displaying split rows.

[Allan Anderson]

 On Oct. 7, 2014, 12:47 p.m., Cristian Oneț wrote:
  I wouldn't wait for a ship it since, as always the patch is huge, I don't 
  want to speak for others but it's really hard for anybody to make a 
  relevant review this way. I know we had our ups and downs so take this 
  advice as a sign that I care enough to not leave this review request 
  unanswered. If you find the patch good just ship it.
  
  Also I support the effort of trying to improve and to avoid duplication but 
  all I see is that 'CSVDialog' keeps getting new members and judging by 
  their names it has become the most complex state machine I've ever seen.
  
  Here are some nitpicks I came up with by looking at the request.
 
 Allan Anderson wrote:
 Thanks Cristian.  As you say, the patch is huge, although that mainly 
 comes from several (nearly all) UI files having changes.  Alvaro mentioned 
 recently that he doesn't use QTDesigner because of its tendency to bloat the 
 files, I suspect rather like Windows Office.  If I were more confident, I too 
 would use a text editor.  In addition, some code I wanted to change was also 
 going to need to be reused, so I extracted it into two separate routines, and 
 the original large routine I was able to dispense with.  I don't foresee any 
 further major changes in the plugin!
 This patch and the recent associated BUG:339044 REVIEW:120260 - Fix CSV 
 import wizard not working correctly on Windows, between them removed 431 
 lines, so I think I've moved in the right direction.

Committed.
remote: This commit is available for viewing at:
remote: http://commits.kde.org/kmymoney/50f7b6c47ad4f326b7cd6b2e0f8746b8391b3810


 On Oct. 7, 2014, 12:47 p.m., Cristian Oneț wrote:
  kmymoney/plugins/csvimport/investmentwizardpage.ui, line 302
  https://git.reviewboard.kde.org/r/120507/diff/1/?file=316711#file316711line302
 
  Do we really need to center all strings? As a translator I found it 
  verry hard to translate the csvimporter because of the many rich text 
  directives. Please take KMyMoney UI as an example.
 
 Allan Anderson wrote:
 You did mention this before, but now I understand to what you are 
 referring.  May be this is another example of Designer bloat.  I might be 
 able to get a simpler result from producing these UI labels via code.  I'll 
 keep my eye on that.

The UI heading labels have been trimmed.  They are still cemtered, but no 
longer HTML-based


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120507/#review68050
---


On Oct. 16, 2014, 12:15 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120507/
 ---
 
 (Updated Oct. 16, 2014, 12:15 p.m.)
 
 
 Review request for KMymoney.
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Improve functionality across distros.
 It was also the intention to save the windows settings, but it was realised 
 that there was a liklihood that the next file to be imported could have quite 
 different column width and number of rows. So, it was decided to  display the 
 whole of the file being imported instead.  
 Also,the UI has been improved to avoid displaying split rows, with all 
 possible rows being displayed, the window height aligning with the rows, and 
 the width displaying complete columns, as far as the screen allowed.  
 Further, I reworked some areas of the code to avoid duplication and improve 
 re-usability.
 
 
 Diffs
 -
 
   kmymoney/plugins/csvimport/lines-datewizardpage.ui 884f3ba 
   kmymoney/plugins/csvimport/separatorwizardpage.ui b6dba9f 
   kmymoney/plugins/csvimport/bankingwizardpage.ui 0d6a4a8 
   kmymoney/plugins/csvimport/csvdialog.h 24abd9a 
   kmymoney/plugins/csvimport/csvdialog.cpp ab20ed4 
   kmymoney/plugins/csvimport/csvdialog.ui 61ab9ce 
   kmymoney/plugins/csvimport/csvimporterplugin.h d2d2f6c 
   kmymoney/plugins/csvimport/csvimporterplugin.cpp 602b4d9 
   kmymoney/plugins/csvimport/introwizardpage.ui a597d56 
   kmymoney/plugins/csvimport/investmentdlg.cpp 5e7b266 
   kmymoney/plugins/csvimport/investmentwizardpage.ui 846836e 
   kmymoney/plugins/csvimport/investprocessing.h fa8ffa0 
   kmymoney/plugins/csvimport/investprocessing.cpp 1629e14 
 
 Diff: https://git.reviewboard.kde.org/r/120507/diff/
 
 
 Testing
 ---
 
 Tested with numerous banking and investment files from different sources, on 
 Linux Mint KDE and Ubuntu Unity.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 120507: CSV Plugin - Improve functionality across distros. Improve UI to display the whole of the file being imported, and smarten to avoid displaying split rows.

[Allan Anderson]

 On Oct. 7, 2014, 12:47 p.m., Cristian Oneț wrote:
  I wouldn't wait for a ship it since, as always the patch is huge, I don't 
  want to speak for others but it's really hard for anybody to make a 
  relevant review this way. I know we had our ups and downs so take this 
  advice as a sign that I care enough to not leave this review request 
  unanswered. If you find the patch good just ship it.
  
  Also I support the effort of trying to improve and to avoid duplication but 
  all I see is that 'CSVDialog' keeps getting new members and judging by 
  their names it has become the most complex state machine I've ever seen.
  
  Here are some nitpicks I came up with by looking at the request.

Thanks Cristian.  As you say, the patch is huge, although that mainly comes 
from several (nearly all) UI files having changes.  Alvaro mentioned recently 
that he doesn't use QTDesigner because of its tendency to bloat the files, I 
suspect rather like Windows Office.  If I were more confident, I too would use 
a text editor.  In addition, some code I wanted to change was also going to 
need to be reused, so I extracted it into two separate routines, and the 
original large routine I was able to dispense with.  I don't foresee any 
further major changes in the plugin!
This patch and the recent associated BUG:339044 REVIEW:120260 - Fix CSV import 
wizard not working correctly on Windows, between them removed 431 lines, so I 
think I've moved in the right direction.


 On Oct. 7, 2014, 12:47 p.m., Cristian Oneț wrote:
  kmymoney/plugins/csvimport/investmentwizardpage.ui, line 302
  https://git.reviewboard.kde.org/r/120507/diff/1/?file=316711#file316711line302
 
  Do we really need to center all strings? As a translator I found it 
  verry hard to translate the csvimporter because of the many rich text 
  directives. Please take KMyMoney UI as an example.

You did mention this before, but now I understand to what you are referring.  
May be this is another example of Designer bloat.  I might be able to get a 
simpler result from producing these UI labels via code.  I'll keep my eye on 
that.


 On Oct. 7, 2014, 12:47 p.m., Cristian Oneț wrote:
  kmymoney/plugins/csvimport/investprocessing.h, line 146
  https://git.reviewboard.kde.org/r/120507/diff/1/?file=316712#file316712line146
 
  Whitespace errors (useless whitespace in empty lines or at the end of 
  line) are pretty well highlighted by reviewboard.

Oops, missed that extra line when lookin through the patch.  Did you say you 
were going to look into using a more up-to-date version of astyle?


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120507/#review68050
---


On Oct. 5, 2014, 6:34 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120507/
 ---
 
 (Updated Oct. 5, 2014, 6:34 p.m.)
 
 
 Review request for KMymoney.
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Improve functionality across distros.
 It was also the intention to save the windows settings, but it was realised 
 that there was a liklihood that the next file to be imported could have quite 
 different column width and number of rows. So, it was decided to  display the 
 whole of the file being imported instead.  
 Also,the UI has been improved to avoid displaying split rows, with all 
 possible rows being displayed, the window height aligning with the rows, and 
 the width displaying complete columns, as far as the screen allowed.  
 Further, I reworked some areas of the code to avoid duplication and improve 
 re-usability.
 
 
 Diffs
 -
 
   kmymoney/plugins/csvimport/lines-datewizardpage.ui 884f3ba 
   kmymoney/plugins/csvimport/separatorwizardpage.ui b6dba9f 
   kmymoney/plugins/csvimport/bankingwizardpage.ui 0d6a4a8 
   kmymoney/plugins/csvimport/csvdialog.h 24abd9a 
   kmymoney/plugins/csvimport/csvdialog.cpp ab20ed4 
   kmymoney/plugins/csvimport/csvdialog.ui 61ab9ce 
   kmymoney/plugins/csvimport/csvimporterplugin.h d2d2f6c 
   kmymoney/plugins/csvimport/csvimporterplugin.cpp 602b4d9 
   kmymoney/plugins/csvimport/introwizardpage.ui a597d56 
   kmymoney/plugins/csvimport/investmentdlg.cpp 5e7b266 
   kmymoney/plugins/csvimport/investmentwizardpage.ui 846836e 
   kmymoney/plugins/csvimport/investprocessing.h fa8ffa0 
   kmymoney/plugins/csvimport/investprocessing.cpp 1629e14 
 
 Diff: https://git.reviewboard.kde.org/r/120507/diff/
 
 
 Testing
 ---
 
 Tested with numerous banking and investment files from different sources, on 
 Linux Mint KDE and Ubuntu Unity.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https

[Kmymoney-devel] Review Request 120507: CSV Plugin - Improve functionality across distros. Improve UI to display the whole of the file being imported, and smarten to avoid displaying split rows. Also

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120507/
---

Review request for KMymoney.


Repository: kmymoney


Description
---

Improve functionality across distros.
It was also the intention to save the windows settings, but it was realised 
that there was a liklihood that the next file to be imported could have quite 
different column width and number of rows. So, it was decided to  display the 
whole of the file being imported instead.  
Also,the UI has been improved to avoid displaying split rows, with all possible 
rows being displayed, the window height aligning with the rows, and the width 
displaying complete columns, as far as the screen allowed.  Further, I reworked 
some areas of the code to avoid duplication and improve re-usability.


Diffs
-

  kmymoney/plugins/csvimport/lines-datewizardpage.ui 884f3ba 
  kmymoney/plugins/csvimport/separatorwizardpage.ui b6dba9f 
  kmymoney/plugins/csvimport/bankingwizardpage.ui 0d6a4a8 
  kmymoney/plugins/csvimport/csvdialog.h 24abd9a 
  kmymoney/plugins/csvimport/csvdialog.cpp ab20ed4 
  kmymoney/plugins/csvimport/csvdialog.ui 61ab9ce 
  kmymoney/plugins/csvimport/csvimporterplugin.h d2d2f6c 
  kmymoney/plugins/csvimport/csvimporterplugin.cpp 602b4d9 
  kmymoney/plugins/csvimport/introwizardpage.ui a597d56 
  kmymoney/plugins/csvimport/investmentdlg.cpp 5e7b266 
  kmymoney/plugins/csvimport/investmentwizardpage.ui 846836e 
  kmymoney/plugins/csvimport/investprocessing.h fa8ffa0 
  kmymoney/plugins/csvimport/investprocessing.cpp 1629e14 

Diff: https://git.reviewboard.kde.org/r/120507/diff/


Testing
---

Tested with numerous banking and investment files from different sources, on 
Linux Mint KDE and Ubuntu Unity.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 120260: CSV import wizard does not appear to be working.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120260/
---

(Updated Sept. 19, 2014, 12:56 p.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Repository: kmymoney


Description
---

I've been working for a while on improving the appearance and operation of the 
CSV importer plugin, which has gone far more slowly than I expected.
In the meantime, a new version of KMyMoney has been released for Windows, and 
two users have reported the known problem of buttons at the foot of the plugin 
page not appearing fully.  Plus, a difficulty with the start page radio 
buttons.  So, there is more urgency now, and I've extracted the reworked UI 
files, and made a patch which includes them, with a needed small change to the 
code handling the UI.  Also, there is a small fix for occasional crashes 
related to decimal symbol handling.  This is based on the current HEAD.  
Otherwise, the UI looks much like the HEAD version, apart from the correct 
visibility of the buttons.
I am still working on improving other parts of the plugin


Diffs
-

  kmymoney/plugins/csvimport/separatorwizardpage.ui 30b2dc7 
  kmymoney/plugins/csvimport/csvdialog.cpp 7e0229b 
  kmymoney/plugins/csvimport/csvdialog.ui 36e7fd5 
  kmymoney/plugins/csvimport/csvutil.h 546bfa5 
  kmymoney/plugins/csvimport/csvutil.cpp 4c0dd25 
  kmymoney/plugins/csvimport/introwizardpage.ui 9bd29f5 
  kmymoney/plugins/csvimport/investmentwizardpage.ui 3744963 
  kmymoney/plugins/csvimport/investprocessing.h 55dd4ca 
  kmymoney/plugins/csvimport/investprocessing.cpp c21c679 
  kmymoney/plugins/csvimport/lines-datewizardpage.ui fb10a0e 
  kmymoney/plugins/csvimport/bankingwizardpage.ui d2179bf 
  kmymoney/plugins/csvimport/completionwizardpage.ui 99db075 
  kmymoney/plugins/csvimport/csvdialog.h b9d4527 

Diff: https://git.reviewboard.kde.org/r/120260/diff/


Testing
---

I've tested the patch on Linux Mint and Ubuntu, with a large number of 
different files.
I am not able to test on Windows, having only XP.
I don't know if these changes help with the radio button issue on Windows, as I 
am not able to reproduce the problem here, the radio, and all other, buttons 
work as expected.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 120260: CSV import wizard does not appear to be working.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120260/
---

Review request for KMymoney.


Repository: kmymoney


Description
---

I've been working for a while on improving the appearance and operation of the 
CSV importer plugin, which has gone far more slowly than I expected.
In the meantime, a new version of KMyMoney has been released for Windows, and 
two users have reported the known problem of buttons at the foot of the plugin 
page not appearing fully.  Plus, a difficulty with the start page radio 
buttons.  So, there is more urgency now, and I've extracted the reworked UI 
files, and made a patch which includes them, with a needed small change to the 
code handling the UI.  Also, there is a small fix for occasional crashes 
related to decimal symbol handling.  This is based on the current HEAD.  
Otherwise, the UI looks much like the HEAD version, apart from the correct 
visibility of the buttons.
I am still working on improving other parts of the plugin


Diffs
-

  kmymoney/plugins/csvimport/separatorwizardpage.ui 30b2dc7 
  kmymoney/plugins/csvimport/csvdialog.cpp 7e0229b 
  kmymoney/plugins/csvimport/csvdialog.ui 36e7fd5 
  kmymoney/plugins/csvimport/csvutil.h 546bfa5 
  kmymoney/plugins/csvimport/csvutil.cpp 4c0dd25 
  kmymoney/plugins/csvimport/introwizardpage.ui 9bd29f5 
  kmymoney/plugins/csvimport/investmentwizardpage.ui 3744963 
  kmymoney/plugins/csvimport/investprocessing.h 55dd4ca 
  kmymoney/plugins/csvimport/investprocessing.cpp c21c679 
  kmymoney/plugins/csvimport/lines-datewizardpage.ui fb10a0e 
  kmymoney/plugins/csvimport/bankingwizardpage.ui d2179bf 
  kmymoney/plugins/csvimport/completionwizardpage.ui 99db075 
  kmymoney/plugins/csvimport/csvdialog.h b9d4527 

Diff: https://git.reviewboard.kde.org/r/120260/diff/


Testing
---

I've tested the patch on Linux Mint and Ubuntu, with a large number of 
different files.
I am not able to test on Windows, having only XP.
I don't know if these changes help with the radio button issue on Windows, as I 
am not able to reproduce the problem here, the radio, and all other, buttons 
work as expected.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

 On April 18, 2014, 11:34 a.m., Allan Anderson wrote:
  Hi Cristian
  Many thanks.  It had been starting to dawn on me that, when dealing with 
  different distros, it shouldn't be necessary to fiddle about with margins
  to get things to look right.  I'd suspected that I needed to make some 
  fundamental changes, but wasn't really ready to take the plunge.  Anyway,
  you've done it for me, so again, thank you.
  The question now is, where do we go from here?
  Here, I'd already removed a chunk of code and have been working on details 
  of the UI appearance.  For instance, I don't like to see the size of the
  table changing height between the different wizard pages, and I don't like 
  to see just partial lines displayed.  Obviously, if the user decides to do
  a resize, then the decision is his, but without that happening, I don't 
  want to see half lines displayed.  This is mainly to do with whether a
  horizontal scroll-bar is visible or not.  I have not been able to get that 
  to work correctly.  Sometimes, it is visible but isVisible() returns
  false, and vice versa I think.  So I've had to calculate the diplayed width 
  and adjust the containing rectangle.  A little work is still needed here.
  In your pruning, I think you have removed some vertical spacers, which is 
  causing the combo-boxes to shift upwards, leaving a lot of space below.
  I wish to revert that.  In the separators wizard page, the two combo-boxes 
  are now different sizes, I think because you've removed the form layout I 
  was
  using.  In the banking wizard, the combo-box sizes are significantly 
  different in width between Linux Mint and Ubuntu.  I don't know about on 
  Windows.
  These are very minor points, but I'd like to tweak them.
  I'd decided on a default window height of 10 rows, which has been changed.  
  Was there a particular reason for that?
  Between Mint and Ubuntu, the margin values are still different, but now 
  that has no effect on the appearance.  The font sizes are differ too, 9 
  point
  against 11 point.  That does affect the appearance.  Should I leave that as 
  is, do you think?  I think probably yes.
  I can't add a revised patch to your review, but if you agree, I can 
  describe any proposed changes.  I don't want to stray away from your major 
  changes,
  though, obviously.
 
 Cristian Oneț wrote:
 where do we go from here?
 
 I don't know, it's up to you to decide. If you ask me (after 2 days of 
 going trough csvdialog.cpp) I would suggest to rewrite everything. I know 
 that seems harsh but frankly I didn't see such tangled up code in a while. 
 One reading it couldn't quite tell what is essential (necessary) in there and 
 what is trial/error leftover code.
 
 For the UI I would have the following guide:
 - keep it simple (your layouts were really, really complicated)
 - don't center everything (including messages)
 - don't set fix sizes (fixed size policies are OK in the appropriate 
 place - line edits, combos should have a fixed vertical size hint)
 - try to use the available space as uniformly as possible (1 page with 2 
 combos vs. 1 page with 10 combos)
 - now we have a dialog (QWizard) inside another dialog (CSVDialog - 
 former widget) which seems a bad idea to me
 
 For the code:
 - don't keep everything together (like a big ball of spaghetti), try to 
 break it up into smaller components
 - only keep the essential code otherwise you won't be able to spot it 
 later from the clutter
 
 Until the csv importer plugin will not be cleaned up, clean reviewable 
 patches can't be provided because the patches will always look like the code 
 that is patched.
 
 Cristian Oneț wrote:
 Forgot to mention the fonts: don't set any fonts - let the user choose 
 the fonts he desires using the system settings.
 
 Allan Anderson wrote:
 I've never made any claim to be a programmer!  This all started off as a 
 script to produce a QIF file, which I was advised to expand, first into
 tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a 
 lot along the way, but I'm still not a programmer.
 In the light of your comments, there seems little point now wasting time 
 tuning what we have, so I'll leave it as is.
 When/if a rewrite occurs will depend upon my available time.
 
 Cristian Oneț wrote:
 Sorry if I offended you, I know to well how one feels about his work made 
 voluntarily in his free time. I'm just asking that you never stop learning 
 and improving the work you do. I even have the same critical view of my own 
 code that I wrote back when I started contributing to kmymoney. I did ask for 
 a rewrite but didn't say when so whenever you feel like it seems fine. 
 Meanwhile we can leave this patch in suspension if it's too much.
 
 Allan Anderson wrote:
 No, your criticism was valid, if a little brusque.
 The patch is needed, so must go forward.
 
  - try

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

 On April 18, 2014, 11:34 a.m., Allan Anderson wrote:
  Hi Cristian
  Many thanks.  It had been starting to dawn on me that, when dealing with 
  different distros, it shouldn't be necessary to fiddle about with margins
  to get things to look right.  I'd suspected that I needed to make some 
  fundamental changes, but wasn't really ready to take the plunge.  Anyway,
  you've done it for me, so again, thank you.
  The question now is, where do we go from here?
  Here, I'd already removed a chunk of code and have been working on details 
  of the UI appearance.  For instance, I don't like to see the size of the
  table changing height between the different wizard pages, and I don't like 
  to see just partial lines displayed.  Obviously, if the user decides to do
  a resize, then the decision is his, but without that happening, I don't 
  want to see half lines displayed.  This is mainly to do with whether a
  horizontal scroll-bar is visible or not.  I have not been able to get that 
  to work correctly.  Sometimes, it is visible but isVisible() returns
  false, and vice versa I think.  So I've had to calculate the diplayed width 
  and adjust the containing rectangle.  A little work is still needed here.
  In your pruning, I think you have removed some vertical spacers, which is 
  causing the combo-boxes to shift upwards, leaving a lot of space below.
  I wish to revert that.  In the separators wizard page, the two combo-boxes 
  are now different sizes, I think because you've removed the form layout I 
  was
  using.  In the banking wizard, the combo-box sizes are significantly 
  different in width between Linux Mint and Ubuntu.  I don't know about on 
  Windows.
  These are very minor points, but I'd like to tweak them.
  I'd decided on a default window height of 10 rows, which has been changed.  
  Was there a particular reason for that?
  Between Mint and Ubuntu, the margin values are still different, but now 
  that has no effect on the appearance.  The font sizes are differ too, 9 
  point
  against 11 point.  That does affect the appearance.  Should I leave that as 
  is, do you think?  I think probably yes.
  I can't add a revised patch to your review, but if you agree, I can 
  describe any proposed changes.  I don't want to stray away from your major 
  changes,
  though, obviously.
 
 Cristian Oneț wrote:
 where do we go from here?
 
 I don't know, it's up to you to decide. If you ask me (after 2 days of 
 going trough csvdialog.cpp) I would suggest to rewrite everything. I know 
 that seems harsh but frankly I didn't see such tangled up code in a while. 
 One reading it couldn't quite tell what is essential (necessary) in there and 
 what is trial/error leftover code.
 
 For the UI I would have the following guide:
 - keep it simple (your layouts were really, really complicated)
 - don't center everything (including messages)
 - don't set fix sizes (fixed size policies are OK in the appropriate 
 place - line edits, combos should have a fixed vertical size hint)
 - try to use the available space as uniformly as possible (1 page with 2 
 combos vs. 1 page with 10 combos)
 - now we have a dialog (QWizard) inside another dialog (CSVDialog - 
 former widget) which seems a bad idea to me
 
 For the code:
 - don't keep everything together (like a big ball of spaghetti), try to 
 break it up into smaller components
 - only keep the essential code otherwise you won't be able to spot it 
 later from the clutter
 
 Until the csv importer plugin will not be cleaned up, clean reviewable 
 patches can't be provided because the patches will always look like the code 
 that is patched.
 
 Cristian Oneț wrote:
 Forgot to mention the fonts: don't set any fonts - let the user choose 
 the fonts he desires using the system settings.
 
 Allan Anderson wrote:
 I've never made any claim to be a programmer!  This all started off as a 
 script to produce a QIF file, which I was advised to expand, first into
 tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a 
 lot along the way, but I'm still not a programmer.
 In the light of your comments, there seems little point now wasting time 
 tuning what we have, so I'll leave it as is.
 When/if a rewrite occurs will depend upon my available time.
 
 Cristian Oneț wrote:
 Sorry if I offended you, I know to well how one feels about his work made 
 voluntarily in his free time. I'm just asking that you never stop learning 
 and improving the work you do. I even have the same critical view of my own 
 code that I wrote back when I started contributing to kmymoney. I did ask for 
 a rewrite but didn't say when so whenever you feel like it seems fine. 
 Meanwhile we can leave this patch in suspension if it's too much.
 
 Allan Anderson wrote:
 No, your criticism was valid, if a little brusque.
 The patch is needed, so must go forward.
 
  - try

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

 On April 18, 2014, 11:34 a.m., Allan Anderson wrote:
  Hi Cristian
  Many thanks.  It had been starting to dawn on me that, when dealing with 
  different distros, it shouldn't be necessary to fiddle about with margins
  to get things to look right.  I'd suspected that I needed to make some 
  fundamental changes, but wasn't really ready to take the plunge.  Anyway,
  you've done it for me, so again, thank you.
  The question now is, where do we go from here?
  Here, I'd already removed a chunk of code and have been working on details 
  of the UI appearance.  For instance, I don't like to see the size of the
  table changing height between the different wizard pages, and I don't like 
  to see just partial lines displayed.  Obviously, if the user decides to do
  a resize, then the decision is his, but without that happening, I don't 
  want to see half lines displayed.  This is mainly to do with whether a
  horizontal scroll-bar is visible or not.  I have not been able to get that 
  to work correctly.  Sometimes, it is visible but isVisible() returns
  false, and vice versa I think.  So I've had to calculate the diplayed width 
  and adjust the containing rectangle.  A little work is still needed here.
  In your pruning, I think you have removed some vertical spacers, which is 
  causing the combo-boxes to shift upwards, leaving a lot of space below.
  I wish to revert that.  In the separators wizard page, the two combo-boxes 
  are now different sizes, I think because you've removed the form layout I 
  was
  using.  In the banking wizard, the combo-box sizes are significantly 
  different in width between Linux Mint and Ubuntu.  I don't know about on 
  Windows.
  These are very minor points, but I'd like to tweak them.
  I'd decided on a default window height of 10 rows, which has been changed.  
  Was there a particular reason for that?
  Between Mint and Ubuntu, the margin values are still different, but now 
  that has no effect on the appearance.  The font sizes are differ too, 9 
  point
  against 11 point.  That does affect the appearance.  Should I leave that as 
  is, do you think?  I think probably yes.
  I can't add a revised patch to your review, but if you agree, I can 
  describe any proposed changes.  I don't want to stray away from your major 
  changes,
  though, obviously.
 
 Cristian Oneț wrote:
 where do we go from here?
 
 I don't know, it's up to you to decide. If you ask me (after 2 days of 
 going trough csvdialog.cpp) I would suggest to rewrite everything. I know 
 that seems harsh but frankly I didn't see such tangled up code in a while. 
 One reading it couldn't quite tell what is essential (necessary) in there and 
 what is trial/error leftover code.
 
 For the UI I would have the following guide:
 - keep it simple (your layouts were really, really complicated)
 - don't center everything (including messages)
 - don't set fix sizes (fixed size policies are OK in the appropriate 
 place - line edits, combos should have a fixed vertical size hint)
 - try to use the available space as uniformly as possible (1 page with 2 
 combos vs. 1 page with 10 combos)
 - now we have a dialog (QWizard) inside another dialog (CSVDialog - 
 former widget) which seems a bad idea to me
 
 For the code:
 - don't keep everything together (like a big ball of spaghetti), try to 
 break it up into smaller components
 - only keep the essential code otherwise you won't be able to spot it 
 later from the clutter
 
 Until the csv importer plugin will not be cleaned up, clean reviewable 
 patches can't be provided because the patches will always look like the code 
 that is patched.
 
 Cristian Oneț wrote:
 Forgot to mention the fonts: don't set any fonts - let the user choose 
 the fonts he desires using the system settings.
 
 Allan Anderson wrote:
 I've never made any claim to be a programmer!  This all started off as a 
 script to produce a QIF file, which I was advised to expand, first into
 tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a 
 lot along the way, but I'm still not a programmer.
 In the light of your comments, there seems little point now wasting time 
 tuning what we have, so I'll leave it as is.
 When/if a rewrite occurs will depend upon my available time.
 
 Cristian Oneț wrote:
 Sorry if I offended you, I know to well how one feels about his work made 
 voluntarily in his free time. I'm just asking that you never stop learning 
 and improving the work you do. I even have the same critical view of my own 
 code that I wrote back when I started contributing to kmymoney. I did ask for 
 a rewrite but didn't say when so whenever you feel like it seems fine. 
 Meanwhile we can leave this patch in suspension if it's too much.
 
 Allan Anderson wrote:
 No, your criticism was valid, if a little brusque.
 The patch is needed, so must go forward.
 
  - try

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

 On April 18, 2014, 11:34 a.m., Allan Anderson wrote:
  Hi Cristian
  Many thanks.  It had been starting to dawn on me that, when dealing with 
  different distros, it shouldn't be necessary to fiddle about with margins
  to get things to look right.  I'd suspected that I needed to make some 
  fundamental changes, but wasn't really ready to take the plunge.  Anyway,
  you've done it for me, so again, thank you.
  The question now is, where do we go from here?
  Here, I'd already removed a chunk of code and have been working on details 
  of the UI appearance.  For instance, I don't like to see the size of the
  table changing height between the different wizard pages, and I don't like 
  to see just partial lines displayed.  Obviously, if the user decides to do
  a resize, then the decision is his, but without that happening, I don't 
  want to see half lines displayed.  This is mainly to do with whether a
  horizontal scroll-bar is visible or not.  I have not been able to get that 
  to work correctly.  Sometimes, it is visible but isVisible() returns
  false, and vice versa I think.  So I've had to calculate the diplayed width 
  and adjust the containing rectangle.  A little work is still needed here.
  In your pruning, I think you have removed some vertical spacers, which is 
  causing the combo-boxes to shift upwards, leaving a lot of space below.
  I wish to revert that.  In the separators wizard page, the two combo-boxes 
  are now different sizes, I think because you've removed the form layout I 
  was
  using.  In the banking wizard, the combo-box sizes are significantly 
  different in width between Linux Mint and Ubuntu.  I don't know about on 
  Windows.
  These are very minor points, but I'd like to tweak them.
  I'd decided on a default window height of 10 rows, which has been changed.  
  Was there a particular reason for that?
  Between Mint and Ubuntu, the margin values are still different, but now 
  that has no effect on the appearance.  The font sizes are differ too, 9 
  point
  against 11 point.  That does affect the appearance.  Should I leave that as 
  is, do you think?  I think probably yes.
  I can't add a revised patch to your review, but if you agree, I can 
  describe any proposed changes.  I don't want to stray away from your major 
  changes,
  though, obviously.
 
 Cristian Oneț wrote:
 where do we go from here?
 
 I don't know, it's up to you to decide. If you ask me (after 2 days of 
 going trough csvdialog.cpp) I would suggest to rewrite everything. I know 
 that seems harsh but frankly I didn't see such tangled up code in a while. 
 One reading it couldn't quite tell what is essential (necessary) in there and 
 what is trial/error leftover code.
 
 For the UI I would have the following guide:
 - keep it simple (your layouts were really, really complicated)
 - don't center everything (including messages)
 - don't set fix sizes (fixed size policies are OK in the appropriate 
 place - line edits, combos should have a fixed vertical size hint)
 - try to use the available space as uniformly as possible (1 page with 2 
 combos vs. 1 page with 10 combos)
 - now we have a dialog (QWizard) inside another dialog (CSVDialog - 
 former widget) which seems a bad idea to me
 
 For the code:
 - don't keep everything together (like a big ball of spaghetti), try to 
 break it up into smaller components
 - only keep the essential code otherwise you won't be able to spot it 
 later from the clutter
 
 Until the csv importer plugin will not be cleaned up, clean reviewable 
 patches can't be provided because the patches will always look like the code 
 that is patched.
 
 Cristian Oneț wrote:
 Forgot to mention the fonts: don't set any fonts - let the user choose 
 the fonts he desires using the system settings.
 
 Allan Anderson wrote:
 I've never made any claim to be a programmer!  This all started off as a 
 script to produce a QIF file, which I was advised to expand, first into
 tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a 
 lot along the way, but I'm still not a programmer.
 In the light of your comments, there seems little point now wasting time 
 tuning what we have, so I'll leave it as is.
 When/if a rewrite occurs will depend upon my available time.
 
 Cristian Oneț wrote:
 Sorry if I offended you, I know to well how one feels about his work made 
 voluntarily in his free time. I'm just asking that you never stop learning 
 and improving the work you do. I even have the same critical view of my own 
 code that I wrote back when I started contributing to kmymoney. I did ask for 
 a rewrite but didn't say when so whenever you feel like it seems fine. 
 Meanwhile we can leave this patch in suspension if it's too much.
 
 Allan Anderson wrote:
 No, your criticism was valid, if a little brusque.
 The patch is needed, so must go forward.
 
  - try

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

 On April 18, 2014, 11:34 a.m., Allan Anderson wrote:
  Hi Cristian
  Many thanks.  It had been starting to dawn on me that, when dealing with 
  different distros, it shouldn't be necessary to fiddle about with margins
  to get things to look right.  I'd suspected that I needed to make some 
  fundamental changes, but wasn't really ready to take the plunge.  Anyway,
  you've done it for me, so again, thank you.
  The question now is, where do we go from here?
  Here, I'd already removed a chunk of code and have been working on details 
  of the UI appearance.  For instance, I don't like to see the size of the
  table changing height between the different wizard pages, and I don't like 
  to see just partial lines displayed.  Obviously, if the user decides to do
  a resize, then the decision is his, but without that happening, I don't 
  want to see half lines displayed.  This is mainly to do with whether a
  horizontal scroll-bar is visible or not.  I have not been able to get that 
  to work correctly.  Sometimes, it is visible but isVisible() returns
  false, and vice versa I think.  So I've had to calculate the diplayed width 
  and adjust the containing rectangle.  A little work is still needed here.
  In your pruning, I think you have removed some vertical spacers, which is 
  causing the combo-boxes to shift upwards, leaving a lot of space below.
  I wish to revert that.  In the separators wizard page, the two combo-boxes 
  are now different sizes, I think because you've removed the form layout I 
  was
  using.  In the banking wizard, the combo-box sizes are significantly 
  different in width between Linux Mint and Ubuntu.  I don't know about on 
  Windows.
  These are very minor points, but I'd like to tweak them.
  I'd decided on a default window height of 10 rows, which has been changed.  
  Was there a particular reason for that?
  Between Mint and Ubuntu, the margin values are still different, but now 
  that has no effect on the appearance.  The font sizes are differ too, 9 
  point
  against 11 point.  That does affect the appearance.  Should I leave that as 
  is, do you think?  I think probably yes.
  I can't add a revised patch to your review, but if you agree, I can 
  describe any proposed changes.  I don't want to stray away from your major 
  changes,
  though, obviously.
 
 Cristian Oneț wrote:
 where do we go from here?
 
 I don't know, it's up to you to decide. If you ask me (after 2 days of 
 going trough csvdialog.cpp) I would suggest to rewrite everything. I know 
 that seems harsh but frankly I didn't see such tangled up code in a while. 
 One reading it couldn't quite tell what is essential (necessary) in there and 
 what is trial/error leftover code.
 
 For the UI I would have the following guide:
 - keep it simple (your layouts were really, really complicated)
 - don't center everything (including messages)
 - don't set fix sizes (fixed size policies are OK in the appropriate 
 place - line edits, combos should have a fixed vertical size hint)
 - try to use the available space as uniformly as possible (1 page with 2 
 combos vs. 1 page with 10 combos)
 - now we have a dialog (QWizard) inside another dialog (CSVDialog - 
 former widget) which seems a bad idea to me
 
 For the code:
 - don't keep everything together (like a big ball of spaghetti), try to 
 break it up into smaller components
 - only keep the essential code otherwise you won't be able to spot it 
 later from the clutter
 
 Until the csv importer plugin will not be cleaned up, clean reviewable 
 patches can't be provided because the patches will always look like the code 
 that is patched.
 
 Cristian Oneț wrote:
 Forgot to mention the fonts: don't set any fonts - let the user choose 
 the fonts he desires using the system settings.
 
 Allan Anderson wrote:
 I've never made any claim to be a programmer!  This all started off as a 
 script to produce a QIF file, which I was advised to expand, first into
 tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a 
 lot along the way, but I'm still not a programmer.
 In the light of your comments, there seems little point now wasting time 
 tuning what we have, so I'll leave it as is.
 When/if a rewrite occurs will depend upon my available time.
 
 Cristian Oneț wrote:
 Sorry if I offended you, I know to well how one feels about his work made 
 voluntarily in his free time. I'm just asking that you never stop learning 
 and improving the work you do. I even have the same critical view of my own 
 code that I wrote back when I started contributing to kmymoney. I did ask for 
 a rewrite but didn't say when so whenever you feel like it seems fine. 
 Meanwhile we can leave this patch in suspension if it's too much.
 
 Allan Anderson wrote:
 No, your criticism was valid, if a little brusque.
 The patch is needed, so must go forward.
 
  - try

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117620/#review56020
---


Hi Cristian
Many thanks.  It had been starting to dawn on me that, when dealing with 
different distros, it shouldn't be necessary to fiddle about with margins
to get things to look right.  I'd suspected that I needed to make some 
fundamental changes, but wasn't really ready to take the plunge.  Anyway,
you've done it for me, so again, thank you.
The question now is, where do we go from here?
Here, I'd already removed a chunk of code and have been working on details of 
the UI appearance.  For instance, I don't like to see the size of the
table changing height between the different wizard pages, and I don't like to 
see just partial lines displayed.  Obviously, if the user decides to do
a resize, then the decision is his, but without that happening, I don't want to 
see half lines displayed.  This is mainly to do with whether a
horizontal scroll-bar is visible or not.  I have not been able to get that to 
work correctly.  Sometimes, it is visible but isVisible() returns
false, and vice versa I think.  So I've had to calculate the diplayed width and 
adjust the containing rectangle.  A little work is still needed here.
In your pruning, I think you have removed some vertical spacers, which is 
causing the combo-boxes to shift upwards, leaving a lot of space below.
I wish to revert that.  In the separators wizard page, the two combo-boxes are 
now different sizes, I think because you've removed the form layout I was
using.  In the banking wizard, the combo-box sizes are significantly different 
in width between Linux Mint and Ubuntu.  I don't know about on Windows.
These are very minor points, but I'd like to tweak them.
I'd decided on a default window height of 10 rows, which has been changed.  Was 
there a particular reason for that?
Between Mint and Ubuntu, the margin values are still different, but now that 
has no effect on the appearance.  The font sizes are differ too, 9 point
against 11 point.  That does affect the appearance.  Should I leave that as is, 
do you think?  I think probably yes.
I can't add a revised patch to your review, but if you agree, I can describe 
any proposed changes.  I don't want to stray away from your major changes,
though, obviously.

- Allan Anderson


On April 17, 2014, 9:23 p.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117620/
 ---
 
 (Updated April 17, 2014, 9:23 p.m.)
 
 
 Review request for KMymoney and Allan Anderson.
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Remove fixed layout values from the CSV importer UI.
 
 Also removed a lot of code (a lot still remains) that I think should
 not be in there (like UI control size fine tuning).
 
 
 Diffs
 -
 
   kmymoney/plugins/csvimport/bankingwizardpage.ui 
 d2179bff0504a67a22efbb364f90e089443d8239 
   kmymoney/plugins/csvimport/completionwizardpage.ui 
 99db07576265b68c764308bbb3da3cae38b151bd 
   kmymoney/plugins/csvimport/csvdialog.h 
 6716ca412db036384d9d8d65f0d1ab40efbe443f 
   kmymoney/plugins/csvimport/csvdialog.cpp 
 3ab7f9537acdb369b0c5b781827564e30d9ad396 
   kmymoney/plugins/csvimport/csvdialog.ui 
 36e7fd51159a1f7391394b5f00bc22387b0bd8f5 
   kmymoney/plugins/csvimport/csvimporterplugin.h 
 d2d2f6ca9aeea0709512afcffbad17abea6a315a 
   kmymoney/plugins/csvimport/csvimporterplugin.cpp 
 602b4d924c8afc0b34819e47b03b88776319bf0c 
   kmymoney/plugins/csvimport/introwizardpage.ui 
 9bd29f5f4339add8790e032f5613ec46c675634d 
   kmymoney/plugins/csvimport/investmentwizardpage.ui 
 3744963e5e9a91ad36b8d0fa78839b296c5810f8 
   kmymoney/plugins/csvimport/investprocessing.cpp 
 d9df90d6b9a6500b499e52ccb3e8734d163765eb 
   kmymoney/plugins/csvimport/lines-datewizardpage.ui 
 fb10a0e228d7bdee9ca1162edcd0d4b69225d68b 
   kmymoney/plugins/csvimport/separatorwizardpage.ui 
 30b2dc7b22ad24b7b2df96332deb25f9c8c76b89 
 
 Diff: https://git.reviewboard.kde.org/r/117620/diff/
 
 
 Testing
 ---
 
 For me the importer looks/feel almost like without this.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

 On April 18, 2014, 11:34 a.m., Allan Anderson wrote:
  Hi Cristian
  Many thanks.  It had been starting to dawn on me that, when dealing with 
  different distros, it shouldn't be necessary to fiddle about with margins
  to get things to look right.  I'd suspected that I needed to make some 
  fundamental changes, but wasn't really ready to take the plunge.  Anyway,
  you've done it for me, so again, thank you.
  The question now is, where do we go from here?
  Here, I'd already removed a chunk of code and have been working on details 
  of the UI appearance.  For instance, I don't like to see the size of the
  table changing height between the different wizard pages, and I don't like 
  to see just partial lines displayed.  Obviously, if the user decides to do
  a resize, then the decision is his, but without that happening, I don't 
  want to see half lines displayed.  This is mainly to do with whether a
  horizontal scroll-bar is visible or not.  I have not been able to get that 
  to work correctly.  Sometimes, it is visible but isVisible() returns
  false, and vice versa I think.  So I've had to calculate the diplayed width 
  and adjust the containing rectangle.  A little work is still needed here.
  In your pruning, I think you have removed some vertical spacers, which is 
  causing the combo-boxes to shift upwards, leaving a lot of space below.
  I wish to revert that.  In the separators wizard page, the two combo-boxes 
  are now different sizes, I think because you've removed the form layout I 
  was
  using.  In the banking wizard, the combo-box sizes are significantly 
  different in width between Linux Mint and Ubuntu.  I don't know about on 
  Windows.
  These are very minor points, but I'd like to tweak them.
  I'd decided on a default window height of 10 rows, which has been changed.  
  Was there a particular reason for that?
  Between Mint and Ubuntu, the margin values are still different, but now 
  that has no effect on the appearance.  The font sizes are differ too, 9 
  point
  against 11 point.  That does affect the appearance.  Should I leave that as 
  is, do you think?  I think probably yes.
  I can't add a revised patch to your review, but if you agree, I can 
  describe any proposed changes.  I don't want to stray away from your major 
  changes,
  though, obviously.
 
 Cristian Oneț wrote:
 where do we go from here?
 
 I don't know, it's up to you to decide. If you ask me (after 2 days of 
 going trough csvdialog.cpp) I would suggest to rewrite everything. I know 
 that seems harsh but frankly I didn't see such tangled up code in a while. 
 One reading it couldn't quite tell what is essential (necessary) in there and 
 what is trial/error leftover code.
 
 For the UI I would have the following guide:
 - keep it simple (your layouts were really, really complicated)
 - don't center everything (including messages)
 - don't set fix sizes (fixed size policies are OK in the appropriate 
 place - line edits, combos should have a fixed vertical size hint)
 - try to use the available space as uniformly as possible (1 page with 2 
 combos vs. 1 page with 10 combos)
 - now we have a dialog (QWizard) inside another dialog (CSVDialog - 
 former widget) which seems a bad idea to me
 
 For the code:
 - don't keep everything together (like a big ball of spaghetti), try to 
 break it up into smaller components
 - only keep the essential code otherwise you won't be able to spot it 
 later from the clutter
 
 Until the csv importer plugin will not be cleaned up, clean reviewable 
 patches can't be provided because the patches will always look like the code 
 that is patched.
 
 Cristian Oneț wrote:
 Forgot to mention the fonts: don't set any fonts - let the user choose 
 the fonts he desires using the system settings.

I've never made any claim to be a programmer!  This all started off as a script 
to produce a QIF file, which I was advised to expand, first into
tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a lot 
along the way, but I'm still not a programmer.
In the light of your comments, there seems little point now wasting time tuning 
what we have, so I'll leave it as is.
When/if a rewrite occurs will depend upon my available time.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117620/#review56020
---


On April 17, 2014, 9:23 p.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117620/
 ---
 
 (Updated April 17, 2014, 9:23 p.m.)
 
 
 Review request for KMymoney and Allan Anderson.
 
 
 Repository: kmymoney
 
 
 Description

Re: [Kmymoney-devel] Review Request 117620: Remove fixed layout values from the CSV importer UI.

[Allan Anderson]

 On April 18, 2014, 11:34 a.m., Allan Anderson wrote:
  Hi Cristian
  Many thanks.  It had been starting to dawn on me that, when dealing with 
  different distros, it shouldn't be necessary to fiddle about with margins
  to get things to look right.  I'd suspected that I needed to make some 
  fundamental changes, but wasn't really ready to take the plunge.  Anyway,
  you've done it for me, so again, thank you.
  The question now is, where do we go from here?
  Here, I'd already removed a chunk of code and have been working on details 
  of the UI appearance.  For instance, I don't like to see the size of the
  table changing height between the different wizard pages, and I don't like 
  to see just partial lines displayed.  Obviously, if the user decides to do
  a resize, then the decision is his, but without that happening, I don't 
  want to see half lines displayed.  This is mainly to do with whether a
  horizontal scroll-bar is visible or not.  I have not been able to get that 
  to work correctly.  Sometimes, it is visible but isVisible() returns
  false, and vice versa I think.  So I've had to calculate the diplayed width 
  and adjust the containing rectangle.  A little work is still needed here.
  In your pruning, I think you have removed some vertical spacers, which is 
  causing the combo-boxes to shift upwards, leaving a lot of space below.
  I wish to revert that.  In the separators wizard page, the two combo-boxes 
  are now different sizes, I think because you've removed the form layout I 
  was
  using.  In the banking wizard, the combo-box sizes are significantly 
  different in width between Linux Mint and Ubuntu.  I don't know about on 
  Windows.
  These are very minor points, but I'd like to tweak them.
  I'd decided on a default window height of 10 rows, which has been changed.  
  Was there a particular reason for that?
  Between Mint and Ubuntu, the margin values are still different, but now 
  that has no effect on the appearance.  The font sizes are differ too, 9 
  point
  against 11 point.  That does affect the appearance.  Should I leave that as 
  is, do you think?  I think probably yes.
  I can't add a revised patch to your review, but if you agree, I can 
  describe any proposed changes.  I don't want to stray away from your major 
  changes,
  though, obviously.
 
 Cristian Oneț wrote:
 where do we go from here?
 
 I don't know, it's up to you to decide. If you ask me (after 2 days of 
 going trough csvdialog.cpp) I would suggest to rewrite everything. I know 
 that seems harsh but frankly I didn't see such tangled up code in a while. 
 One reading it couldn't quite tell what is essential (necessary) in there and 
 what is trial/error leftover code.
 
 For the UI I would have the following guide:
 - keep it simple (your layouts were really, really complicated)
 - don't center everything (including messages)
 - don't set fix sizes (fixed size policies are OK in the appropriate 
 place - line edits, combos should have a fixed vertical size hint)
 - try to use the available space as uniformly as possible (1 page with 2 
 combos vs. 1 page with 10 combos)
 - now we have a dialog (QWizard) inside another dialog (CSVDialog - 
 former widget) which seems a bad idea to me
 
 For the code:
 - don't keep everything together (like a big ball of spaghetti), try to 
 break it up into smaller components
 - only keep the essential code otherwise you won't be able to spot it 
 later from the clutter
 
 Until the csv importer plugin will not be cleaned up, clean reviewable 
 patches can't be provided because the patches will always look like the code 
 that is patched.
 
 Cristian Oneț wrote:
 Forgot to mention the fonts: don't set any fonts - let the user choose 
 the fonts he desires using the system settings.
 
 Allan Anderson wrote:
 I've never made any claim to be a programmer!  This all started off as a 
 script to produce a QIF file, which I was advised to expand, first into
 tabs, and later into a wizard.  It has grown, like Topsy.  I've learned a 
 lot along the way, but I'm still not a programmer.
 In the light of your comments, there seems little point now wasting time 
 tuning what we have, so I'll leave it as is.
 When/if a rewrite occurs will depend upon my available time.
 
 Cristian Oneț wrote:
 Sorry if I offended you, I know to well how one feels about his work made 
 voluntarily in his free time. I'm just asking that you never stop learning 
 and improving the work you do. I even have the same critical view of my own 
 code that I wrote back when I started contributing to kmymoney. I did ask for 
 a rewrite but didn't say when so whenever you feel like it seems fine. 
 Meanwhile we can leave this patch in suspension if it's too much.

No, your criticism was valid, if a little brusque.
The patch is needed, so must go forward.

 - try to use the available space as uniformly

Re: [Kmymoney-devel] Review Request 115338: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115338/
---

(Updated Feb. 17, 2014, 1:08 p.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

Follow-up patch on https://git.reviewboard.kde.org/r/115302/,
 to fix handling of number field.
There was no check that a new 'next number' did not already exist.  For 
instance, if there is the sequence 22,23,24, and the user entered a new 
transaction numbered 21, the 'next number' would have been 22, which already 
exists.
Deletion of a numered transaction would produce an incorrect next number.
An edit of an existing number was not saved.
These issues, which exist also in the stable release, have been fixed.
(new Reviewboard item raised for this part, on suggestion of Thomas Baumgart).


Diffs
-

  kmymoney/kmymoneyutils.cpp e89528e 
  kmymoney/kmymoneyutils.h a899121 
  kmymoney/kmymoney.cpp 7a2fd4f 
  kmymoney/dialogs/transactioneditor.cpp 3c148d8 
  kmymoney/dialogs/transactioneditor.h db86832 

Diff: https://git.reviewboard.kde.org/r/115338/diff/


Testing
---

Numerous manual test cases run.  Unit tests run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 115302: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115302/
---

(Updated Jan. 27, 2014, 11:31 a.m.)


Review request for KMymoney.


Changes
---

Follow-up patch to fix handling of number field.
There was no check that a new 'next number' did not already exist.  For 
instance, if there is the sequence 22,23,24, and the user entered a new 
transaction numbered 21, the 'next number' would have been 22, which already 
exists.
Deletion of a numered transaction would produce an incorrect next number.
An edit of an existing number was not saved.
These issues, which exist also in the stable release, have been fixed,


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  These issues, which exist also in the current release, also have been 
fixed, but are not included in this Review, for clarity.  They will be 
submitted separately later.

The values in the unit test which are not purely numeric can now be used, and 
produce correct 'next numbers'.


Diffs (updated)
-

  kmymoney/kmymoneyutils.cpp e89528e 
  kmymoney/kmymoneyutils.h a899121 
  kmymoney/kmymoney.cpp 7a2fd4f 
  kmymoney/dialogs/transactioneditor.cpp 3c148d8 
  kmymoney/dialogs/transactioneditor.h db86832 

Diff: https://git.reviewboard.kde.org/r/115302/diff/


Testing
---

Numerous manual test cases run.  Unit tests run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 115338: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115338/
---

Review request for KMymoney.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

Follow-up patch on https://git.reviewboard.kde.org/r/115302/,
 to fix handling of number field.
There was no check that a new 'next number' did not already exist.  For 
instance, if there is the sequence 22,23,24, and the user entered a new 
transaction numbered 21, the 'next number' would have been 22, which already 
exists.
Deletion of a numered transaction would produce an incorrect next number.
An edit of an existing number was not saved.
These issues, which exist also in the stable release, have been fixed.
(new Reviewboard item raised for this part, on suggestion of Thomas Baumgart).


Diffs
-

  kmymoney/kmymoneyutils.cpp e89528e 
  kmymoney/kmymoneyutils.h a899121 
  kmymoney/kmymoney.cpp 7a2fd4f 
  kmymoney/dialogs/transactioneditor.cpp 3c148d8 
  kmymoney/dialogs/transactioneditor.h db86832 

Diff: https://git.reviewboard.kde.org/r/115338/diff/


Testing
---

Numerous manual test cases run.  Unit tests run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 115302: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115302/
---

(Updated Jan. 27, 2014, 5:33 p.m.)


Status
--

This change has been discarded.


Review request for KMymoney.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  These issues, which exist also in the current release, also have been 
fixed, but are not included in this Review, for clarity.  They will be 
submitted separately later.

The values in the unit test which are not purely numeric can now be used, and 
produce correct 'next numbers'.


Diffs
-

  kmymoney/kmymoneyutils.cpp e89528e 
  kmymoney/kmymoneyutils.h a899121 
  kmymoney/kmymoney.cpp 7a2fd4f 
  kmymoney/dialogs/transactioneditor.cpp 3c148d8 
  kmymoney/dialogs/transactioneditor.h db86832 

Diff: https://git.reviewboard.kde.org/r/115302/diff/


Testing
---

Numerous manual test cases run.  Unit tests run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 115302: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115302/
---

(Updated Jan. 26, 2014, 10:53 a.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  These issues, which exist also in the current release, also have been 
fixed, but are not included in this Review, for clarity.  They will be 
submitted separately later.

The values in the unit test which are not purely numeric can now be used, and 
produce correct 'next numbers'.


Diffs
-

  kmymoney/kmymoneyutils.cpp 7058557 
  kmymoney/kmymoneyutils.h f64a55e 
  kmymoney/dialogs/transactioneditor.cpp 26e7672 
  kmymoney/dialogs/transactioneditor.h 25705a0 

Diff: https://git.reviewboard.kde.org/r/115302/diff/


Testing
---

Numerous manual test cases run.  Unit tests run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113143/#review48234
---


This has been in Review for a while.  Also, it looks a bit of a mess now, so 
I'll withdraw it and re-submit the cleaned up version.

- Allan Anderson


On Oct. 20, 2013, 7:15 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/113143/
 ---
 
 (Updated Oct. 20, 2013, 7:15 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 319801
 http://bugs.kde.org/show_bug.cgi?id=319801
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
 number such as 'No 123-001 ABC', the next check number produced will be '1', 
 entries containing alpha or punctuation characters not being saved.
 The fix corrects this by saving the complete entry, and uses any entered 
 numeric part to calculate the next number in sequence.  If an existing 
 numeric entry is edited, this entry will be taken into account for the next 
 number.
 
 There are some quirks.  If the entry which led to the current 'next number' 
 is deleted, it is not possible to revert to the previous, now forgotten, 
 'next number', so the produced 'next number' is likely to leave a 'gap', and 
 may need editing.  Also, there is no check that a new 'next number' does not 
 already exist.  For instance, if there is the erroneous sequence 23,23,24, 
 the 'next number' will be the expected 25.  However, if the user corrects the 
 error by changing a 23 to 22, the new 'next number' will be 23, which also 
 already exists.  I'm not sure if such issues, which exist also in the current 
 release, are worthy of fixing for a fairly unimportant area, without becoming 
 more involved.
 
 
 Diffs
 -
 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 71d94ec 
   kmymoney/kmymoneyutils.h f64a55e 
   kmymoney/kmymoneyutils.cpp 7058557 
 
 Diff: https://git.reviewboard.kde.org/r/113143/diff/
 
 
 Testing
 ---
 
 Many manual entries checked, including coping with all values in the unit 
 tests.  The unit test runs OK.
 
 
 File Attachments
 
 
 Part 2
   
 https://git.reviewboard.kde.org/media/uploaded/files/2013/10/20/7aac2cee-e36f-466f-995f-d80c526094c3__0001-BUG-319801_REVIEW113143_-_Fix_losing_track_of_check_numberB.patch
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113143/
---

(Updated Jan. 24, 2014, 4:51 p.m.)


Status
--

This change has been discarded.


Review request for KMymoney.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  I'm not sure if such issues, which exist also in the current release, 
are worthy of fixing for a fairly unimportant area, without becoming more 
involved.


Diffs
-

  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp 71d94ec 
  kmymoney/kmymoneyutils.h f64a55e 
  kmymoney/kmymoneyutils.cpp 7058557 

Diff: https://git.reviewboard.kde.org/r/113143/diff/


Testing
---

Many manual entries checked, including coping with all values in the unit 
tests.  The unit test runs OK.


File Attachments


Part 2
  
https://git.reviewboard.kde.org/media/uploaded/files/2013/10/20/7aac2cee-e36f-466f-995f-d80c526094c3__0001-BUG-319801_REVIEW113143_-_Fix_losing_track_of_check_numberB.patch


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 115302: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115302/
---

Review request for KMymoney.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  These issues, which exist also in the current release, also have been 
fixed, but are not included in this Review, for clarity.  They will be 
submitted separately later.

The values in the unit test which are not purely numeric can now be used, and 
produce correct 'next numbers'.


Diffs
-

  kmymoney/kmymoneyutils.cpp 7058557 
  kmymoney/kmymoneyutils.h f64a55e 
  kmymoney/dialogs/transactioneditor.cpp 26e7672 
  kmymoney/dialogs/transactioneditor.h 25705a0 

Diff: https://git.reviewboard.kde.org/r/115302/diff/


Testing
---

Numerous manual test cases run.  Unit tests run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 114352: Bug 312816 - Fine tune the resizing of the register and the transaction form.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114352/
---

(Updated Jan. 7, 2014, 11:28 a.m.)


Status
--

This change has been discarded.


Review request for KMymoney and Cristian Oneț.


Bugs: 312816
http://bugs.kde.org/show_bug.cgi?id=312816


Repository: kmymoney


Description
---

This patch extends the code of REVIEW: 112989, and as I cannot add a patch to 
Cristian's review request, I have raised a new request.

a) When an account is loaded, the columns for the kMyMoneyEdit fields have 
extra white-space, to allow for the calculator and date buttons, but these 
buttons won't appear until editing starts.  This happens because 
m_usedWithEditor was being enabled in KGlobalLedgerView ctor. I've changed this 
so m_usedWithEditor is enabled instead in KGlobalLedgerView::startEdit() and 
reset in KGlobalLedgerView::slotLeaveEditMode().

b) When a transaction is opened for editing, the above columns are widened for 
the buttons.  However, this window widening is not helped by potentially 
surplus white-space appearing in the Description column.  This can frequently 
happen if the memo text is wide and possibly originally multi-line.  When not 
editing, the multi-lines are combined into a wider field, but the multi-lines 
are restored when editing but the Details column does not notice this, so 
retains its full width.  I've remedied this by reducing the width to that of 
the widest sub-string.


Diffs
-

  kmymoney/views/kgloballedgerview.cpp 000393c 
  kmymoney/widgets/register.cpp 56bf46d 

Diff: https://git.reviewboard.kde.org/r/114352/diff/


Testing
---

Much resizing of many checking and investment accounts, both while in edit mode 
and not.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113427: Editing mulitple transactions - cannot change Memo field

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113427/#review46743
---


- Allan Anderson


On Oct. 24, 2013, 11:06 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/113427/
 ---
 
 (Updated Oct. 24, 2013, 11:06 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 289351
 http://bugs.kde.org/show_bug.cgi?id=289351
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 There was a previous fix for this problem - Git commit 
 9485826cfb50816d2df4dac9709b4beb255b8b75 by Cristian One?. Unfortunately, 
 this was inadvertently disabled when BUG:311481 REVIEW:107714 was committed.  
 This is now fixed.
 This review adds the same capability for investment transactions.
 In addition, there have been requests that when clearing the memo field, it 
 should when required be empty rather that containing a blank character.  It 
 is necessary to enter a character in the memo field in order to signal that 
 an edit has occurred, but now that character may then be deleted to leave the 
 field empty, if that is what is required.
 With investment transactions, it is always necessary to enter the security 
 name as well. This requirement could probably be removed, but it is probably 
 sensible to be editing just a single security.
 As with all editing of multiple items, all fields appear as blank initially. 
 This has been retained here with multiple memo editing, but if desired, this 
 could be changed and the field could be left showing previous content, which 
 could be more intuitive for a user wanting to have an empty field.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.h aa4800f 
   kmymoney/dialogs/investactivities.cpp e4760e5 
   kmymoney/dialogs/investtransactioneditor.h 20e3819 
   kmymoney/dialogs/investtransactioneditor.cpp 805bd8d 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 71d94ec 
 
 Diff: https://git.reviewboard.kde.org/r/113427/diff/
 
 
 Testing
 ---
 
 Numerous groups of investment and checking transactions entered and edited 
 correctly.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113427: Editing mulitple transactions - cannot change Memo field

[Allan Anderson]

 On Jan. 2, 2014, 12:19 a.m., Allan Anderson wrote:
  I've noticed a potential problem.  When making a multi-edit, the memo field 
  appears cleared.  If any other edit is made, and the edit is entered, the 
  original memos are cleared too, which is undesirable.
  So, what I've done is remove the initial clearing of the memo fields, so 
  their original content is maintained.  As it happens, I made this 
  suggestion in the review - As with all editing of multiple items, all 
  fields appear as blank initially. This has been retained here with multiple 
  memo editing, but if desired, this could be changed and the field could be 
  left showing previous content, which could be more intuitive for a user 
  wanting to have an empty field.
  I hope this is acceptable.  It also avoids the requirement for a user who 
  wants an empty memo, to have to enter a character into the empty memo field 
  in order to delete it to trigger a connect.
 
 Thomas Baumgart wrote:
 I am not sure, if that is a good and practical solution. Let's say you 
 have the following scenario:
 
 - User selects two transactions containing different memo content
 - User inadvertently changes the memo field and uses 'Undo' to undo the 
 change
 - Now he changes another field and enters the transactions
 
 What happens to the memo content of the other transaction? Is it changed? 
 It shouldn't. That is why I think leaving the field blank in the first place 
 is the better choice. We can add a What's this? or Tooltip to the memo 
 edit field which contains the hint that deleting the contents is achieved by 
 adding a single blank.

Let's say I have two transactions, and do a multi-edit on them, to add a memo 
comment of Memo.  OK so far.
Then I edit transaction A and add to the memo string MemoB, so A now has 
MemoMemoA, then change B to MemoMemoB.
Then I do another multi-edit, and remove the second string.  Interestingly, 
both keep their first string.
Next I repeat the above, except I change my mind and do an undo then enter.  So 
now they are back to MemoMemoA and MemoMemoB.
This time,  I repeat the above, but, without entering, I now edit the date 
jointly, then enter.  The memos still show MemoMemoA and MemoMemoB, so 
there appears to be no adverse effect.
I'll do some more playing about. Assuming nothing seems amiss, am I OK still to 
Ship?


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113427/#review46590
---


On Oct. 24, 2013, 11:06 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/113427/
 ---
 
 (Updated Oct. 24, 2013, 11:06 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 289351
 http://bugs.kde.org/show_bug.cgi?id=289351
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 There was a previous fix for this problem - Git commit 
 9485826cfb50816d2df4dac9709b4beb255b8b75 by Cristian One?. Unfortunately, 
 this was inadvertently disabled when BUG:311481 REVIEW:107714 was committed.  
 This is now fixed.
 This review adds the same capability for investment transactions.
 In addition, there have been requests that when clearing the memo field, it 
 should when required be empty rather that containing a blank character.  It 
 is necessary to enter a character in the memo field in order to signal that 
 an edit has occurred, but now that character may then be deleted to leave the 
 field empty, if that is what is required.
 With investment transactions, it is always necessary to enter the security 
 name as well. This requirement could probably be removed, but it is probably 
 sensible to be editing just a single security.
 As with all editing of multiple items, all fields appear as blank initially. 
 This has been retained here with multiple memo editing, but if desired, this 
 could be changed and the field could be left showing previous content, which 
 could be more intuitive for a user wanting to have an empty field.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.h aa4800f 
   kmymoney/dialogs/investactivities.cpp e4760e5 
   kmymoney/dialogs/investtransactioneditor.h 20e3819 
   kmymoney/dialogs/investtransactioneditor.cpp 805bd8d 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 71d94ec 
 
 Diff: https://git.reviewboard.kde.org/r/113427/diff/
 
 
 Testing
 ---
 
 Numerous groups of investment and checking transactions entered and edited 
 correctly.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113427: Editing mulitple transactions - cannot change Memo field

[Allan Anderson]

 On Jan. 2, 2014, 12:19 a.m., Allan Anderson wrote:
  I've noticed a potential problem.  When making a multi-edit, the memo field 
  appears cleared.  If any other edit is made, and the edit is entered, the 
  original memos are cleared too, which is undesirable.
  So, what I've done is remove the initial clearing of the memo fields, so 
  their original content is maintained.  As it happens, I made this 
  suggestion in the review - As with all editing of multiple items, all 
  fields appear as blank initially. This has been retained here with multiple 
  memo editing, but if desired, this could be changed and the field could be 
  left showing previous content, which could be more intuitive for a user 
  wanting to have an empty field.
  I hope this is acceptable.  It also avoids the requirement for a user who 
  wants an empty memo, to have to enter a character into the empty memo field 
  in order to delete it to trigger a connect.
 
 Thomas Baumgart wrote:
 I am not sure, if that is a good and practical solution. Let's say you 
 have the following scenario:
 
 - User selects two transactions containing different memo content
 - User inadvertently changes the memo field and uses 'Undo' to undo the 
 change
 - Now he changes another field and enters the transactions
 
 What happens to the memo content of the other transaction? Is it changed? 
 It shouldn't. That is why I think leaving the field blank in the first place 
 is the better choice. We can add a What's this? or Tooltip to the memo 
 edit field which contains the hint that deleting the contents is achieved by 
 adding a single blank.
 
 Allan Anderson wrote:
 Let's say I have two transactions, and do a multi-edit on them, to add a 
 memo comment of Memo.  OK so far.
 Then I edit transaction A and add to the memo string MemoB, so A now 
 has MemoMemoA, then change B to MemoMemoB.
 Then I do another multi-edit, and remove the second string.  
 Interestingly, both keep their first string.
 Next I repeat the above, except I change my mind and do an undo then 
 enter.  So now they are back to MemoMemoA and MemoMemoB.
 This time,  I repeat the above, but, without entering, I now edit the 
 date jointly, then enter.  The memos still show MemoMemoA and MemoMemoB, 
 so there appears to be no adverse effect.
 I'll do some more playing about. Assuming nothing seems amiss, am I OK 
 still to Ship?

There is at least one difference between how Investment and Standard 
transactions react in multi-edit mode.  With Investments, it's possible to edit 
the date, whereas that's not possible with Standard transactions.
Assuming it's desirable that they perform in similar fashion, which would you 
prefer?  My initial thought is that it is unlikely that anyone would want to 
edit the dates to be the same on several transactions.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113427/#review46590
---


On Oct. 24, 2013, 11:06 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/113427/
 ---
 
 (Updated Oct. 24, 2013, 11:06 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 289351
 http://bugs.kde.org/show_bug.cgi?id=289351
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 There was a previous fix for this problem - Git commit 
 9485826cfb50816d2df4dac9709b4beb255b8b75 by Cristian One?. Unfortunately, 
 this was inadvertently disabled when BUG:311481 REVIEW:107714 was committed.  
 This is now fixed.
 This review adds the same capability for investment transactions.
 In addition, there have been requests that when clearing the memo field, it 
 should when required be empty rather that containing a blank character.  It 
 is necessary to enter a character in the memo field in order to signal that 
 an edit has occurred, but now that character may then be deleted to leave the 
 field empty, if that is what is required.
 With investment transactions, it is always necessary to enter the security 
 name as well. This requirement could probably be removed, but it is probably 
 sensible to be editing just a single security.
 As with all editing of multiple items, all fields appear as blank initially. 
 This has been retained here with multiple memo editing, but if desired, this 
 could be changed and the field could be left showing previous content, which 
 could be more intuitive for a user wanting to have an empty field.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.h aa4800f 
   kmymoney/dialogs/investactivities.cpp e4760e5 
   kmymoney/dialogs/investtransactioneditor.h 20e3819 
   kmymoney/dialogs/investtransactioneditor.cpp 805bd8d

Re: [Kmymoney-devel] Review Request 113427: Editing mulitple transactions - cannot change Memo field

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113427/#review46590
---


I've noticed a potential problem.  When making a multi-edit, the memo field 
appears cleared.  If any other edit is made, and the edit is entered, the 
original memos are cleared too, which is undesirable.
So, what I've done is remove the initial clearing of the memo fields, so their 
original content is maintained.  As it happens, I made this suggestion in the 
review - As with all editing of multiple items, all fields appear as blank 
initially. This has been retained here with multiple memo editing, but if 
desired, this could be changed and the field could be left showing previous 
content, which could be more intuitive for a user wanting to have an empty 
field.
I hope this is acceptable.  It also avoids the requirement for a user who wants 
an empty memo, to have to enter a character into the empty memo field in order 
to delete it to trigger a connect.

- Allan Anderson


On Oct. 24, 2013, 11:06 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/113427/
 ---
 
 (Updated Oct. 24, 2013, 11:06 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 289351
 http://bugs.kde.org/show_bug.cgi?id=289351
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 There was a previous fix for this problem - Git commit 
 9485826cfb50816d2df4dac9709b4beb255b8b75 by Cristian One?. Unfortunately, 
 this was inadvertently disabled when BUG:311481 REVIEW:107714 was committed.  
 This is now fixed.
 This review adds the same capability for investment transactions.
 In addition, there have been requests that when clearing the memo field, it 
 should when required be empty rather that containing a blank character.  It 
 is necessary to enter a character in the memo field in order to signal that 
 an edit has occurred, but now that character may then be deleted to leave the 
 field empty, if that is what is required.
 With investment transactions, it is always necessary to enter the security 
 name as well. This requirement could probably be removed, but it is probably 
 sensible to be editing just a single security.
 As with all editing of multiple items, all fields appear as blank initially. 
 This has been retained here with multiple memo editing, but if desired, this 
 could be changed and the field could be left showing previous content, which 
 could be more intuitive for a user wanting to have an empty field.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.h aa4800f 
   kmymoney/dialogs/investactivities.cpp e4760e5 
   kmymoney/dialogs/investtransactioneditor.h 20e3819 
   kmymoney/dialogs/investtransactioneditor.cpp 805bd8d 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 71d94ec 
 
 Diff: https://git.reviewboard.kde.org/r/113427/diff/
 
 
 Testing
 ---
 
 Numerous groups of investment and checking transactions entered and edited 
 correctly.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113427: Editing mulitple transactions - cannot change Memo field

[Allan Anderson]

 On Jan. 1, 2014, 2:40 p.m., Thomas Baumgart wrote:
  kmymoney/dialogs/investtransactioneditor.cpp, line 1170
  https://git.reviewboard.kde.org/r/113427/diff/1/?file=205822#file205822line1170
 
  Isn't that simply
  
  if(memo) {
d-m_activity-m_memoChanged = (memo-toPlainText() != 
  d-m_activity-m_memoText);
  }

done


 On Jan. 1, 2014, 2:40 p.m., Thomas Baumgart wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 278
  https://git.reviewboard.kde.org/r/113427/diff/1/?file=205824#file205824line278
 
  You should also check for memo being a NULL ptr here just like in 
  InvestTransactionEditor::slotUpdateInvestMemoState(void)

Of course. Done


- Allan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/113427/#review46543
---


On Oct. 24, 2013, 11:06 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/113427/
 ---
 
 (Updated Oct. 24, 2013, 11:06 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 289351
 http://bugs.kde.org/show_bug.cgi?id=289351
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 There was a previous fix for this problem - Git commit 
 9485826cfb50816d2df4dac9709b4beb255b8b75 by Cristian One?. Unfortunately, 
 this was inadvertently disabled when BUG:311481 REVIEW:107714 was committed.  
 This is now fixed.
 This review adds the same capability for investment transactions.
 In addition, there have been requests that when clearing the memo field, it 
 should when required be empty rather that containing a blank character.  It 
 is necessary to enter a character in the memo field in order to signal that 
 an edit has occurred, but now that character may then be deleted to leave the 
 field empty, if that is what is required.
 With investment transactions, it is always necessary to enter the security 
 name as well. This requirement could probably be removed, but it is probably 
 sensible to be editing just a single security.
 As with all editing of multiple items, all fields appear as blank initially. 
 This has been retained here with multiple memo editing, but if desired, this 
 could be changed and the field could be left showing previous content, which 
 could be more intuitive for a user wanting to have an empty field.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.h aa4800f 
   kmymoney/dialogs/investactivities.cpp e4760e5 
   kmymoney/dialogs/investtransactioneditor.h 20e3819 
   kmymoney/dialogs/investtransactioneditor.cpp 805bd8d 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 71d94ec 
 
 Diff: https://git.reviewboard.kde.org/r/113427/diff/
 
 
 Testing
 ---
 
 Numerous groups of investment and checking transactions entered and edited 
 correctly.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/#review42006
---


Please ignore diff r3.  I've obviously sent the wrong patch.  Apologies.

- Allan Anderson


On Oct. 19, 2013, 4:53 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113143/
 ---
 
 (Updated Oct. 19, 2013, 4:53 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 319801
 http://bugs.kde.org/show_bug.cgi?id=319801
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
 number such as 'No 123-001 ABC', the next check number produced will be '1', 
 entries containing alpha or punctuation characters not being saved.
 The fix corrects this by saving the complete entry, and uses any entered 
 numeric part to calculate the next number in sequence.  If an existing 
 numeric entry is edited, this entry will be taken into account for the next 
 number.
 
 There are some quirks.  If the entry which led to the current 'next number' 
 is deleted, it is not possible to revert to the previous, now forgotten, 
 'next number', so the produced 'next number' is likely to leave a 'gap', and 
 may need editing.  Also, there is no check that a new 'next number' does not 
 already exist.  For instance, if there is the erroneous sequence 23,23,24, 
 the 'next number' will be the expected 25.  However, if the user corrects the 
 error by changing a 23 to 22, the new 'next number' will be 23, which also 
 already exists.  I'm not sure if such issues, which exist also in the current 
 release, are worthy of fixing for a fairly unimportant area, without becoming 
 more involved.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp cfb0f71 
   kmymoney/widgets/transactioneditorcontainer.h f77b195 
 
 Diff: http://git.reviewboard.kde.org/r/113143/diff/
 
 
 Testing
 ---
 
 Many manual entries checked, including coping with all values in the unit 
 tests.  The unit test runs OK.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/
---

(Updated Oct. 20, 2013, 4 p.m.)


Review request for KMymoney.


Changes
---

This is the correct patch for the initial fixes. To avoid extra complexity, 
there will be a further, small, patch dealing with numbering following the 
deletion of a transaction.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  I'm not sure if such issues, which exist also in the current release, 
are worthy of fixing for a fairly unimportant area, without becoming more 
involved.


Diffs (updated)
-

  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp 71d94ec 
  kmymoney/kmymoneyutils.h f64a55e 
  kmymoney/kmymoneyutils.cpp 7058557 

Diff: http://git.reviewboard.kde.org/r/113143/diff/


Testing
---

Many manual entries checked, including coping with all values in the unit 
tests.  The unit test runs OK.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/
---

(Updated Oct. 20, 2013, 7:15 p.m.)


Review request for KMymoney.


Changes
---

This is the second part patch, to install on top of part 1.  It deals with 
check numbering after a transaction deletion.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  I'm not sure if such issues, which exist also in the current release, 
are worthy of fixing for a fairly unimportant area, without becoming more 
involved.


Diffs
-

  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp 71d94ec 
  kmymoney/kmymoneyutils.h f64a55e 
  kmymoney/kmymoneyutils.cpp 7058557 

Diff: http://git.reviewboard.kde.org/r/113143/diff/


Testing
---

Many manual entries checked, including coping with all values in the unit 
tests.  The unit test runs OK.


File Attachments (updated)


Part 2
  
http://git.reviewboard.kde.org/media/uploaded/files/2013/10/20/7aac2cee-e36f-466f-995f-d80c526094c3__0001-BUG-319801_REVIEW113143_-_Fix_losing_track_of_check_numberB.patch


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/#review42035
---



File Attachment: Part 2 - 0001-BUG-319801 REVIEW:113143 - Fix losing track of 
check numberB.patch
http://git.reviewboard.kde.org//r/113143/#fcomment120
This is the second patch, for which I did an 'add file', and which possibly 
I should not have done.


File Attachment: Part 2 - 0001-BUG-319801 REVIEW:113143 - Fix losing track of 
check numberB.patch
http://git.reviewboard.kde.org//r/113143/#fcomment121
This is the second patch, for which I did an 'add file', and which possibly 
I should not have done.


File Attachment: Part 2 - 0001-BUG-319801 REVIEW:113143 - Fix losing track of 
check numberB.patch
http://git.reviewboard.kde.org//r/113143/#fcomment122
This is the second patch, for which I did an 'add file', and which possibly 
I should not have done.

- Allan Anderson


On Oct. 20, 2013, 7:15 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113143/
 ---
 
 (Updated Oct. 20, 2013, 7:15 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 319801
 http://bugs.kde.org/show_bug.cgi?id=319801
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
 number such as 'No 123-001 ABC', the next check number produced will be '1', 
 entries containing alpha or punctuation characters not being saved.
 The fix corrects this by saving the complete entry, and uses any entered 
 numeric part to calculate the next number in sequence.  If an existing 
 numeric entry is edited, this entry will be taken into account for the next 
 number.
 
 There are some quirks.  If the entry which led to the current 'next number' 
 is deleted, it is not possible to revert to the previous, now forgotten, 
 'next number', so the produced 'next number' is likely to leave a 'gap', and 
 may need editing.  Also, there is no check that a new 'next number' does not 
 already exist.  For instance, if there is the erroneous sequence 23,23,24, 
 the 'next number' will be the expected 25.  However, if the user corrects the 
 error by changing a 23 to 22, the new 'next number' will be 23, which also 
 already exists.  I'm not sure if such issues, which exist also in the current 
 release, are worthy of fixing for a fairly unimportant area, without becoming 
 more involved.
 
 
 Diffs
 -
 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 71d94ec 
   kmymoney/kmymoneyutils.h f64a55e 
   kmymoney/kmymoneyutils.cpp 7058557 
 
 Diff: http://git.reviewboard.kde.org/r/113143/diff/
 
 
 Testing
 ---
 
 Many manual entries checked, including coping with all values in the unit 
 tests.  The unit test runs OK.
 
 
 File Attachments
 
 
 Part 2
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/10/20/7aac2cee-e36f-466f-995f-d80c526094c3__0001-BUG-319801_REVIEW113143_-_Fix_losing_track_of_check_numberB.patch
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/
---

(Updated Oct. 19, 2013, 4:53 p.m.)


Review request for KMymoney.


Changes
---

In addressing the issues that followed adding the code to fix the basic 
problem, the handling of check numbers that included alpha or punctuation 
characters, the size of the patch grew somewhat.
For clarity, I've therefore split the patch into two, this part adding the 
capability to handle non-numeric characters.
The second part will follow, which builds upon this first part, and deals with 
the better handling of editing or deleting existing numbers while producing the 
revised next check number.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  I'm not sure if such issues, which exist also in the current release, 
are worthy of fixing for a fairly unimportant area, without becoming more 
involved.


Diffs (updated)
-

  kmymoney/dialogs/investactivities.cpp 50f33ed 
  kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
  kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp cfb0f71 
  kmymoney/widgets/transactioneditorcontainer.h f77b195 

Diff: http://git.reviewboard.kde.org/r/113143/diff/


Testing
---

Many manual entries checked, including coping with all values in the unit 
tests.  The unit test runs OK.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112989: Improve the resizing of the register and the transaction form.

[Allan Anderson]

 On Oct. 11, 2013, 11:53 a.m., Allan Anderson wrote:
  Can you hold for a little while, please.  I think I've found a problem.
 
 Allan Anderson wrote:
 I think the problem with the column widths is to do with the extra space 
 that is needed when a transaction is opened for editing.  That extra space is 
 being retained, resulting in the column widths being unnecessarily wide when 
 not being edited.
 
 The column width is controlled by 'if (m_usedWithEditor  
 !KMyMoneyGlobalSettings::transactionForm())' at line 1149 in
 void Register::resize(int col, bool force).
 
 m_usedWithEditor is enabled on line 204 in 
 KGlobalLedgerView::KGlobalLedgerView(QWidget *parent, const char *name) and 
 then seems to stay that way.  I've patched it instead to be enabled in 
 TransactionEditor* KGlobalLedgerView::startEdit(const 
 KMyMoneyRegister::SelectedTransactions list) and reset in void 
 KGlobalLedgerView::slotLeaveEditMode(const 
 KMyMoneyRegister::SelectedTransactions list).
 
 This allows the columns to shrink when not editing.  It is how I 
 controlled it in my earlier patches.

Yes, the problem I saw was that initially the column widths were too wide 
because space had been created for the edit buttons before a transaction had 
been opened for editing, because m_usedWithEditor was being enabled in 
KGlobalLedgerView ctor. I've changed this so m_usedWithEditor is enabled in 
KGlobalLedgerView::startEdit() and reset in 
KGlobalLedgerView::slotLeaveEditMode().

After this, there was a further problem as, after a transaction had been opened 
for editing and the column widths increased for the buttons, when the 
transaction was closed, there was no final resizing to shrink the columns to 
normal size.  I've patched this by adding the variable bool m_resizing to allow 
this final resize to take place.

I can't see any way for me to add a new patch to your review so I'll add it to 
the bug.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112989/#review41555
---


On Sept. 29, 2013, 11:37 a.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112989/
 ---
 
 (Updated Sept. 29, 2013, 11:37 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 312816
 http://bugs.kde.org/show_bug.cgi?id=312816
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Improve the resizing of the register and the transaction form.
 
 The transaction form was not correctly resizing the Value2 column
 when the data of the KMyMoneyEdit exceeded the size of the widget.
 The size of the push button (when present) was not considered when
 the width was computed based on the cell text.
 
 The same issues were fixed when for the inline transaction editor.
 
 The register resizing was improved in the following ways:
 - the number fields is no longer limited
 - the details column can no longer be shrinked to a size smaller then
 it's needed to render the data it contains
 
 BUG: 312816
 
 
 Diffs
 -
 
   kmymoney/widgets/kmymoneydateinput.cpp 
 856efaa9ddffcec7440cf3530349568d2c789333 
   kmymoney/widgets/register.h 50ce7598783b49197de6acf4859a0fbd7c5c8962 
   kmymoney/widgets/register.cpp d5dd63951d35b7098c0797fe64f10baf07a77fe1 
   kmymoney/widgets/transactionform.cpp 
 642e98f97eb98a14676186c296c1b27972f195cd 
 
 Diff: http://git.reviewboard.kde.org/r/112989/diff/
 
 
 Testing
 ---
 
 Ledger resize with/without the transaction form with multiple account types.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112989: Improve the resizing of the register and the transaction form.

[Allan Anderson]

 On Oct. 11, 2013, 11:53 a.m., Allan Anderson wrote:
  Can you hold for a little while, please.  I think I've found a problem.
 
 Allan Anderson wrote:
 I think the problem with the column widths is to do with the extra space 
 that is needed when a transaction is opened for editing.  That extra space is 
 being retained, resulting in the column widths being unnecessarily wide when 
 not being edited.
 
 The column width is controlled by 'if (m_usedWithEditor  
 !KMyMoneyGlobalSettings::transactionForm())' at line 1149 in
 void Register::resize(int col, bool force).
 
 m_usedWithEditor is enabled on line 204 in 
 KGlobalLedgerView::KGlobalLedgerView(QWidget *parent, const char *name) and 
 then seems to stay that way.  I've patched it instead to be enabled in 
 TransactionEditor* KGlobalLedgerView::startEdit(const 
 KMyMoneyRegister::SelectedTransactions list) and reset in void 
 KGlobalLedgerView::slotLeaveEditMode(const 
 KMyMoneyRegister::SelectedTransactions list).
 
 This allows the columns to shrink when not editing.  It is how I 
 controlled it in my earlier patches.
 
 Allan Anderson wrote:
 Yes, the problem I saw was that initially the column widths were too wide 
 because space had been created for the edit buttons before a transaction had 
 been opened for editing, because m_usedWithEditor was being enabled in 
 KGlobalLedgerView ctor. I've changed this so m_usedWithEditor is enabled in 
 KGlobalLedgerView::startEdit() and reset in 
 KGlobalLedgerView::slotLeaveEditMode().
 
 After this, there was a further problem as, after a transaction had been 
 opened for editing and the column widths increased for the buttons, when the 
 transaction was closed, there was no final resizing to shrink the columns to 
 normal size.  I've patched this by adding the variable bool m_resizing to 
 allow this final resize to take place.
 
 I can't see any way for me to add a new patch to your review so I'll add 
 it to the bug.
 


I've added my patch to the bug you've quoted, but I think you should actually 
have referred to Bug 325341 - Improve the column sizes auto adjustment of the 
register when the transaction form is disabled and the register is in read only 
mode. This improves register column sizes in the Payee, Tags views and in the 
transaction search and autofill dialogs.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112989/#review41555
---


On Sept. 29, 2013, 11:37 a.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112989/
 ---
 
 (Updated Sept. 29, 2013, 11:37 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 312816
 http://bugs.kde.org/show_bug.cgi?id=312816
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Improve the resizing of the register and the transaction form.
 
 The transaction form was not correctly resizing the Value2 column
 when the data of the KMyMoneyEdit exceeded the size of the widget.
 The size of the push button (when present) was not considered when
 the width was computed based on the cell text.
 
 The same issues were fixed when for the inline transaction editor.
 
 The register resizing was improved in the following ways:
 - the number fields is no longer limited
 - the details column can no longer be shrinked to a size smaller then
 it's needed to render the data it contains
 
 BUG: 312816
 
 
 Diffs
 -
 
   kmymoney/widgets/kmymoneydateinput.cpp 
 856efaa9ddffcec7440cf3530349568d2c789333 
   kmymoney/widgets/register.h 50ce7598783b49197de6acf4859a0fbd7c5c8962 
   kmymoney/widgets/register.cpp d5dd63951d35b7098c0797fe64f10baf07a77fe1 
   kmymoney/widgets/transactionform.cpp 
 642e98f97eb98a14676186c296c1b27972f195cd 
 
 Diff: http://git.reviewboard.kde.org/r/112989/diff/
 
 
 Testing
 ---
 
 Ledger resize with/without the transaction form with multiple account types.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112989: Improve the resizing of the register and the transaction form.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112989/#review41548
---

Ship it!


Yes, that looks good for the number and detail columns.
I'm still thinking about users with small screens and the excess width of 
several other columns.  Any objections to me looking into this?
Also, I think I'd found a couple of bunches of code that served no apparent 
purpose (duplication).

- Allan Anderson


On Sept. 29, 2013, 11:37 a.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112989/
 ---
 
 (Updated Sept. 29, 2013, 11:37 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 312816
 http://bugs.kde.org/show_bug.cgi?id=312816
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Improve the resizing of the register and the transaction form.
 
 The transaction form was not correctly resizing the Value2 column
 when the data of the KMyMoneyEdit exceeded the size of the widget.
 The size of the push button (when present) was not considered when
 the width was computed based on the cell text.
 
 The same issues were fixed when for the inline transaction editor.
 
 The register resizing was improved in the following ways:
 - the number fields is no longer limited
 - the details column can no longer be shrinked to a size smaller then
 it's needed to render the data it contains
 
 BUG: 312816
 
 
 Diffs
 -
 
   kmymoney/widgets/kmymoneydateinput.cpp 
 856efaa9ddffcec7440cf3530349568d2c789333 
   kmymoney/widgets/register.h 50ce7598783b49197de6acf4859a0fbd7c5c8962 
   kmymoney/widgets/register.cpp d5dd63951d35b7098c0797fe64f10baf07a77fe1 
   kmymoney/widgets/transactionform.cpp 
 642e98f97eb98a14676186c296c1b27972f195cd 
 
 Diff: http://git.reviewboard.kde.org/r/112989/diff/
 
 
 Testing
 ---
 
 Ledger resize with/without the transaction form with multiple account types.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112989: Improve the resizing of the register and the transaction form.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112989/#review41555
---


Can you hold for a little while, please.  I think I've found a problem.

- Allan Anderson


On Sept. 29, 2013, 11:37 a.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112989/
 ---
 
 (Updated Sept. 29, 2013, 11:37 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 312816
 http://bugs.kde.org/show_bug.cgi?id=312816
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Improve the resizing of the register and the transaction form.
 
 The transaction form was not correctly resizing the Value2 column
 when the data of the KMyMoneyEdit exceeded the size of the widget.
 The size of the push button (when present) was not considered when
 the width was computed based on the cell text.
 
 The same issues were fixed when for the inline transaction editor.
 
 The register resizing was improved in the following ways:
 - the number fields is no longer limited
 - the details column can no longer be shrinked to a size smaller then
 it's needed to render the data it contains
 
 BUG: 312816
 
 
 Diffs
 -
 
   kmymoney/widgets/kmymoneydateinput.cpp 
 856efaa9ddffcec7440cf3530349568d2c789333 
   kmymoney/widgets/register.h 50ce7598783b49197de6acf4859a0fbd7c5c8962 
   kmymoney/widgets/register.cpp d5dd63951d35b7098c0797fe64f10baf07a77fe1 
   kmymoney/widgets/transactionform.cpp 
 642e98f97eb98a14676186c296c1b27972f195cd 
 
 Diff: http://git.reviewboard.kde.org/r/112989/diff/
 
 
 Testing
 ---
 
 Ledger resize with/without the transaction form with multiple account types.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112989: Improve the resizing of the register and the transaction form.

[Allan Anderson]

 On Oct. 11, 2013, 11:53 a.m., Allan Anderson wrote:
  Can you hold for a little while, please.  I think I've found a problem.

I think the problem with the column widths is to do with the extra space that 
is needed when a transaction is opened for editing.  That extra space is being 
retained, resulting in the column widths being unnecessarily wide when not 
being edited.

The column width is controlled by 'if (m_usedWithEditor  
!KMyMoneyGlobalSettings::transactionForm())' at line 1149 in
void Register::resize(int col, bool force).

m_usedWithEditor is enabled on line 204 in 
KGlobalLedgerView::KGlobalLedgerView(QWidget *parent, const char *name) and 
then seems to stay that way.  I've patched it instead to be enabled in 
TransactionEditor* KGlobalLedgerView::startEdit(const 
KMyMoneyRegister::SelectedTransactions list) and reset in void 
KGlobalLedgerView::slotLeaveEditMode(const 
KMyMoneyRegister::SelectedTransactions list).

This allows the columns to shrink when not editing.  It is how I controlled it 
in my earlier patches.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112989/#review41555
---


On Sept. 29, 2013, 11:37 a.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112989/
 ---
 
 (Updated Sept. 29, 2013, 11:37 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 312816
 http://bugs.kde.org/show_bug.cgi?id=312816
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 Improve the resizing of the register and the transaction form.
 
 The transaction form was not correctly resizing the Value2 column
 when the data of the KMyMoneyEdit exceeded the size of the widget.
 The size of the push button (when present) was not considered when
 the width was computed based on the cell text.
 
 The same issues were fixed when for the inline transaction editor.
 
 The register resizing was improved in the following ways:
 - the number fields is no longer limited
 - the details column can no longer be shrinked to a size smaller then
 it's needed to render the data it contains
 
 BUG: 312816
 
 
 Diffs
 -
 
   kmymoney/widgets/kmymoneydateinput.cpp 
 856efaa9ddffcec7440cf3530349568d2c789333 
   kmymoney/widgets/register.h 50ce7598783b49197de6acf4859a0fbd7c5c8962 
   kmymoney/widgets/register.cpp d5dd63951d35b7098c0797fe64f10baf07a77fe1 
   kmymoney/widgets/transactionform.cpp 
 642e98f97eb98a14676186c296c1b27972f195cd 
 
 Diff: http://git.reviewboard.kde.org/r/112989/diff/
 
 
 Testing
 ---
 
 Ledger resize with/without the transaction form with multiple account types.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

 On Oct. 10, 2013, 5:02 a.m., Cristian Oneț wrote:
 

Revised patch to follow.  I've also attempted to fix the 'quirks' I referred to 
above, but I'll keep them separate from this, for clarity.


 On Oct. 10, 2013, 5:02 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 722
  http://git.reviewboard.kde.org/r/113143/diff/1/?file=199630#file199630line722
 
  Please declare variables at the point of their initialization, there is 
  no point in declaring the number variable outside of the if statement. And 
  use a const reference if you don't intend to change it.
  
  const QString number = tr.splits().front().number();

On being passed to keepNewNumber(), 'number' was getting changed so couldn't be 
const.
I've reworked that area anyway, and dispensed with QString number.


 On Oct. 10, 2013, 5:02 a.m., Cristian Oneț wrote:
  kmymoney/mymoney/mymoneyfile.h, line 1480
  http://git.reviewboard.kde.org/r/113143/diff/1/?file=199631#file199631line1480
 
  Please pass a 'const QString ' as a parameter instead of by-value.
  And why does 'strNumericPart' only forward to the 'numericPart' private 
  function, why don't you make 'numericPart' public and remove 
  'strNumericPart'?

 Please pass a 'const QString ' as a parameter instead of by-value. - Done.

I did originally have 'numericPart' as public, but had an impression that 
member variables were best not public so changed it.  Lack of formal tuition 
rears its head again. - Done.


On Oct. 10, 2013, 5:02 a.m., Allan Anderson wrote:
  The above are just some plain old C++ coding style suggestions. Also should 
  'numericPart' really belong to the mymoneyfile object? If so I would add 
  some testcases for it in the mymoneyfiletest suite.

No, not really.  I placed it there only because there were a couple of of other 
similar routines there, but they aren't really that similar.  As it's used only 
by TransactionEditor, I've moved it to there.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/#review41476
---


On Oct. 7, 2013, 8:10 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/113143/
 ---
 
 (Updated Oct. 7, 2013, 8:10 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 319801
 http://bugs.kde.org/show_bug.cgi?id=319801
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
 number such as 'No 123-001 ABC', the next check number produced will be '1', 
 entries containing alpha or punctuation characters not being saved.
 The fix corrects this by saving the complete entry, and uses any entered 
 numeric part to calculate the next number in sequence.  If an existing 
 numeric entry is edited, this entry will be taken into account for the next 
 number.
 
 There are some quirks.  If the entry which led to the current 'next number' 
 is deleted, it is not possible to revert to the previous, now forgotten, 
 'next number', so the produced 'next number' is likely to leave a 'gap', and 
 may need editing.  Also, there is no check that a new 'next number' does not 
 already exist.  For instance, if there is the erroneous sequence 23,23,24, 
 the 'next number' will be the expected 25.  However, if the user corrects the 
 error by changing a 23 to 22, the new 'next number' will be 23, which also 
 already exists.  I'm not sure if such issues, which exist also in the current 
 release, are worthy of fixing for a fairly unimportant area, without becoming 
 more involved.
 
 
 Diffs
 -
 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/mymoney/mymoneyfile.h af0c6fb 
   kmymoney/mymoney/mymoneyfile.cpp ff7302c 
 
 Diff: http://git.reviewboard.kde.org/r/113143/diff/
 
 
 Testing
 ---
 
 Many manual entries checked, including coping with all values in the unit 
 tests.  The unit test runs OK.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 113143: Fix loosing track of check number if check number sequence is interrupted.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113143/
---

Review request for KMymoney.


Bugs: 319801
http://bugs.kde.org/show_bug.cgi?id=319801


Repository: kmymoney


Description
---

If a user's sequence of check numbers is broken by, say 'ATM' or an invoice 
number such as 'No 123-001 ABC', the next check number produced will be '1', 
entries containing alpha or punctuation characters not being saved.
The fix corrects this by saving the complete entry, and uses any entered 
numeric part to calculate the next number in sequence.  If an existing numeric 
entry is edited, this entry will be taken into account for the next number.

There are some quirks.  If the entry which led to the current 'next number' is 
deleted, it is not possible to revert to the previous, now forgotten, 'next 
number', so the produced 'next number' is likely to leave a 'gap', and may need 
editing.  Also, there is no check that a new 'next number' does not already 
exist.  For instance, if there is the erroneous sequence 23,23,24, the 'next 
number' will be the expected 25.  However, if the user corrects the error by 
changing a 23 to 22, the new 'next number' will be 23, which also already 
exists.  I'm not sure if such issues, which exist also in the current release, 
are worthy of fixing for a fairly unimportant area, without becoming more 
involved.


Diffs
-

  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp 39049cf 
  kmymoney/mymoney/mymoneyfile.h af0c6fb 
  kmymoney/mymoney/mymoneyfile.cpp ff7302c 

Diff: http://git.reviewboard.kde.org/r/113143/diff/


Testing
---

Many manual entries checked, including coping with all values in the unit 
tests.  The unit test runs OK.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/
---

(Updated Oct. 3, 2013, 10:23 a.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Bugs: 322768
http://bugs.kde.org/show_bug.cgi?id=322768


Repository: kmymoney


Description
---

- Interest category disappears -
Steps to Reproduce:
1.Open a new Dividend transaction.
2.Enter an interest category and amount.
3.Enter a new fee category.
4.On accepting the new category, the interest category and amount have been 
cleared.
One line fix in kmymoney/dialogs/transactioneditor.cpp.

- Fixes for KEditWidget visibility issues.
 When editing an investment transaction, interest or fee edit widgets show or 
hide incorrectly.  This is a fairly long-standing issue, and there have been 
attempts to fix by delaying the show() or hide() instructions.  This became 
more pronounced during work to allow column resizing, and Cristian produced a 
fix for the root cause.  This fix is included here.
With the above fix in place, it became necessary to revert the delayed show() 
and hide() calls, and this has been done.
Of course, nothing is ever as simple as that, and another couple of issues 
emerged.  Whether or not an interest or fee amount widget is shown depends on 
the presence or absence of the associated category's text.  It transpired that 
if, say, an existing Reinvest transaction was edited to be, say a Buy 
transaction,  the text from the Reinvest interest category was seen by the new 
Buy entry and resulted in the interest-amount widget being visible when none 
should appear.  Similarly, if a transaction with a fee set is edited to be a 
type with no fee expected, for instance, an Add or RemoveShares, then the 
fee-amount widget became visible when not needed.
It was necessary to rework the slotUpdateFeeVisibility() and 
slotUpdateInterestVisibility() functions to take account of the new transaction 
type.


Diffs
-

  kmymoney/dialogs/investactivities.cpp 50f33ed 
  kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
  kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
  kmymoney/dialogs/transactioneditor.cpp cfb0f71 
  kmymoney/widgets/transactioneditorcontainer.h f77b195 

Diff: http://git.reviewboard.kde.org/r/112947/diff/


Testing
---

Entering and editing various transaction types to ensure only the appropriate 
widgets became visible or hidden when required.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

 On Oct. 2, 2013, 4:31 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 1402
  http://git.reviewboard.kde.org/r/112947/diff/2/?file=193217#file193217line1402
 
  Why are you undoing this change I've made some days ago? 
  https://projects.kde.org/projects/extragear/office/kmymoney/repository/revisions/092f3a79f4ec74d25e0f5a4b9c80dfef64db5da3
  
  Are you sure that you have the patch against master?

Cristian, I'm truly sorry.  I'd upgraded my system, and with it came astyle 
2.01.  When I ran astyle, it totally swamped my working directory with changes 
( 500) and became useless.  I think one of the parameters was wrong or 
missing. I started again from master, and applied the first patch.  For my 
later changes I resorted to the pre-astyle files, totally failing to appreciate 
that you had changes to some of those files in master, and which therefore were 
over-written.
I'm really sorry, and must try now to unscramble things properly.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/#review41094
---


On Oct. 1, 2013, 7:33 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112947/
 ---
 
 (Updated Oct. 1, 2013, 7:33 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 322768
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 - Interest category disappears -
 Steps to Reproduce:
 1.Open a new Dividend transaction.
 2.Enter an interest category and amount.
 3.Enter a new fee category.
 4.On accepting the new category, the interest category and amount have been 
 cleared.
 One line fix in kmymoney/dialogs/transactioneditor.cpp.
 
 - Fixes for KEditWidget visibility issues.
  When editing an investment transaction, interest or fee edit widgets show or 
 hide incorrectly.  This is a fairly long-standing issue, and there have been 
 attempts to fix by delaying the show() or hide() instructions.  This became 
 more pronounced during work to allow column resizing, and Cristian produced a 
 fix for the root cause.  This fix is included here.
 With the above fix in place, it became necessary to revert the delayed show() 
 and hide() calls, and this has been done.
 Of course, nothing is ever as simple as that, and another couple of issues 
 emerged.  Whether or not an interest or fee amount widget is shown depends on 
 the presence or absence of the associated category's text.  It transpired 
 that if, say, an existing Reinvest transaction was edited to be, say a Buy 
 transaction,  the text from the Reinvest interest category was seen by the 
 new Buy entry and resulted in the interest-amount widget being visible when 
 none should appear.  Similarly, if a transaction with a fee set is edited to 
 be a type with no fee expected, for instance, an Add or RemoveShares, then 
 the fee-amount widget became visible when not needed.
 It was necessary to rework the slotUpdateFeeVisibility() and 
 slotUpdateInterestVisibility() functions to take account of the new 
 transaction type.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.cpp cfb0f71 
   kmymoney/widgets/kmymoneydateinput.cpp 856efaa 
   kmymoney/widgets/register.h 50ce759 
   kmymoney/widgets/register.cpp d5dd639 
   kmymoney/widgets/transactioneditorcontainer.h f77b195 
   kmymoney/widgets/transactionform.cpp 642e98f 
 
 Diff: http://git.reviewboard.kde.org/r/112947/diff/
 
 
 Testing
 ---
 
 Entering and editing various transaction types to ensure only the appropriate 
 widgets became visible or hidden when required.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

On Oct. 2, 2013, 4:31 a.m., Allan Anderson wrote:
  It seems that there was a problem in the way you created the new patch. You 
  mixed in some other patches and undid some recents commits with it so it 
  can't be shipped unless you fix that.
 
 Cristian Oneț wrote:
 No problem, I mean the patch is fine, just that it contains more then it 
 should. I agree that it's a pain to keep working with the old version of 
 astyle. I'll have a look at how we could update to the never version.

The unholy mess resolved, revised patch to follow.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/#review41094
---


On Oct. 1, 2013, 7:33 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112947/
 ---
 
 (Updated Oct. 1, 2013, 7:33 p.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 322768
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 - Interest category disappears -
 Steps to Reproduce:
 1.Open a new Dividend transaction.
 2.Enter an interest category and amount.
 3.Enter a new fee category.
 4.On accepting the new category, the interest category and amount have been 
 cleared.
 One line fix in kmymoney/dialogs/transactioneditor.cpp.
 
 - Fixes for KEditWidget visibility issues.
  When editing an investment transaction, interest or fee edit widgets show or 
 hide incorrectly.  This is a fairly long-standing issue, and there have been 
 attempts to fix by delaying the show() or hide() instructions.  This became 
 more pronounced during work to allow column resizing, and Cristian produced a 
 fix for the root cause.  This fix is included here.
 With the above fix in place, it became necessary to revert the delayed show() 
 and hide() calls, and this has been done.
 Of course, nothing is ever as simple as that, and another couple of issues 
 emerged.  Whether or not an interest or fee amount widget is shown depends on 
 the presence or absence of the associated category's text.  It transpired 
 that if, say, an existing Reinvest transaction was edited to be, say a Buy 
 transaction,  the text from the Reinvest interest category was seen by the 
 new Buy entry and resulted in the interest-amount widget being visible when 
 none should appear.  Similarly, if a transaction with a fee set is edited to 
 be a type with no fee expected, for instance, an Add or RemoveShares, then 
 the fee-amount widget became visible when not needed.
 It was necessary to rework the slotUpdateFeeVisibility() and 
 slotUpdateInterestVisibility() functions to take account of the new 
 transaction type.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.cpp cfb0f71 
   kmymoney/widgets/kmymoneydateinput.cpp 856efaa 
   kmymoney/widgets/register.h 50ce759 
   kmymoney/widgets/register.cpp d5dd639 
   kmymoney/widgets/transactioneditorcontainer.h f77b195 
   kmymoney/widgets/transactionform.cpp 642e98f 
 
 Diff: http://git.reviewboard.kde.org/r/112947/diff/
 
 
 Testing
 ---
 
 Entering and editing various transaction types to ensure only the appropriate 
 widgets became visible or hidden when required.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/
---

(Updated Oct. 2, 2013, 4:06 p.m.)


Review request for KMymoney.


Changes
---

Removed errors included in last patch.
Very minor format and white-space issues included.
Tested again and double-checked.


Bugs: 322768
http://bugs.kde.org/show_bug.cgi?id=322768


Repository: kmymoney


Description
---

- Interest category disappears -
Steps to Reproduce:
1.Open a new Dividend transaction.
2.Enter an interest category and amount.
3.Enter a new fee category.
4.On accepting the new category, the interest category and amount have been 
cleared.
One line fix in kmymoney/dialogs/transactioneditor.cpp.

- Fixes for KEditWidget visibility issues.
 When editing an investment transaction, interest or fee edit widgets show or 
hide incorrectly.  This is a fairly long-standing issue, and there have been 
attempts to fix by delaying the show() or hide() instructions.  This became 
more pronounced during work to allow column resizing, and Cristian produced a 
fix for the root cause.  This fix is included here.
With the above fix in place, it became necessary to revert the delayed show() 
and hide() calls, and this has been done.
Of course, nothing is ever as simple as that, and another couple of issues 
emerged.  Whether or not an interest or fee amount widget is shown depends on 
the presence or absence of the associated category's text.  It transpired that 
if, say, an existing Reinvest transaction was edited to be, say a Buy 
transaction,  the text from the Reinvest interest category was seen by the new 
Buy entry and resulted in the interest-amount widget being visible when none 
should appear.  Similarly, if a transaction with a fee set is edited to be a 
type with no fee expected, for instance, an Add or RemoveShares, then the 
fee-amount widget became visible when not needed.
It was necessary to rework the slotUpdateFeeVisibility() and 
slotUpdateInterestVisibility() functions to take account of the new transaction 
type.


Diffs (updated)
-

  kmymoney/dialogs/investactivities.cpp 50f33ed 
  kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
  kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
  kmymoney/dialogs/transactioneditor.cpp cfb0f71 
  kmymoney/widgets/transactioneditorcontainer.h f77b195 

Diff: http://git.reviewboard.kde.org/r/112947/diff/


Testing
---

Entering and editing various transaction types to ensure only the appropriate 
widgets became visible or hidden when required.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 464
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192725#file192725line464
 
  Are you sure that this is valid since   categoryId.clear() is 
  called above? This only keeps the data in the widget.
 
 Allan Anderson wrote:
 Yes it is.  As explained above, the data in the widget is the name of the 
 interest category, but we're dealing now with a new fee category.  If we 
 clear this text, the interest category has lost its name and it needs to be 
 added again once the fee category dialog closes.
 It definitely fixes the issue, but whether there is something deeper, I 
 don't know.
 
 Cristian Oneț wrote:
 See my question above, when accepting the transaction what will be the 
 value of 'categoryId' if it has been cleared and only the name was kept? Is 
 it being recomputed based on the name which was kept?
 
 Allan Anderson wrote:
 'categoryId' is empty on entry, in these circumstances - when adding a 
 new category.  Editing an existing one doesn't come here.
 
 Cristian Oneț wrote:
 OK, then it's safe.

Not pushing, but want to be certain whether that means OK to ship, or just 
you're OK with that particular point?  Didn't want it to be in limbo.
Tidied up and retested.  All looks OK with no flicker.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/#review40849
---


On Sept. 26, 2013, 12:03 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112947/
 ---
 
 (Updated Sept. 26, 2013, 12:03 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 322768
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 - Interest category disappears -
 Steps to Reproduce:
 1.Open a new Dividend transaction.
 2.Enter an interest category and amount.
 3.Enter a new fee category.
 4.On accepting the new category, the interest category and amount have been 
 cleared.
 One line fix in kmymoney/dialogs/transactioneditor.cpp.
 
 - Fixes for KEditWidget visibility issues.
  When editing an investment transaction, interest or fee edit widgets show or 
 hide incorrectly.  This is a fairly long-standing issue, and there have been 
 attempts to fix by delaying the show() or hide() instructions.  This became 
 more pronounced during work to allow column resizing, and Cristian produced a 
 fix for the root cause.  This fix is included here.
 With the above fix in place, it became necessary to revert the delayed show() 
 and hide() calls, and this has been done.
 Of course, nothing is ever as simple as that, and another couple of issues 
 emerged.  Whether or not an interest or fee amount widget is shown depends on 
 the presence or absence of the associated category's text.  It transpired 
 that if, say, an existing Reinvest transaction was edited to be, say a Buy 
 transaction,  the text from the Reinvest interest category was seen by the 
 new Buy entry and resulted in the interest-amount widget being visible when 
 none should appear.  Similarly, if a transaction with a fee set is edited to 
 be a type with no fee expected, for instance, an Add or RemoveShares, then 
 the fee-amount widget became visible when not needed.
 It was necessary to rework the slotUpdateFeeVisibility() and 
 slotUpdateInterestVisibility() functions to take account of the new 
 transaction type.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/widgets/transactioneditorcontainer.h f77b195 
 
 Diff: http://git.reviewboard.kde.org/r/112947/diff/
 
 
 Testing
 ---
 
 Entering and editing various transaction types to ensure only the appropriate 
 widgets became visible or hidden when required.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 464
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192725#file192725line464
 
  Are you sure that this is valid since   categoryId.clear() is 
  called above? This only keeps the data in the widget.
 
 Allan Anderson wrote:
 Yes it is.  As explained above, the data in the widget is the name of the 
 interest category, but we're dealing now with a new fee category.  If we 
 clear this text, the interest category has lost its name and it needs to be 
 added again once the fee category dialog closes.
 It definitely fixes the issue, but whether there is something deeper, I 
 don't know.
 
 Cristian Oneț wrote:
 See my question above, when accepting the transaction what will be the 
 value of 'categoryId' if it has been cleared and only the name was kept? Is 
 it being recomputed based on the name which was kept?
 
 Allan Anderson wrote:
 'categoryId' is empty on entry, in these circumstances - when adding a 
 new category.  Editing an existing one doesn't come here.
 
 Cristian Oneț wrote:
 OK, then it's safe.
 
 Allan Anderson wrote:
 Not pushing, but want to be certain whether that means OK to ship, or 
 just you're OK with that particular point?  Didn't want it to be in limbo.
 Tidied up and retested.  All looks OK with no flicker.

 
 Cristian Oneț wrote:
 I was waiting for the updated patch to fix the other issues we've 
 discussed in this review request to give this patch the 'Ship it!'.

Sorry to be dim, but which issues do you mean?  If you mean the column width 
issues, I thought your patch was all you wanted on that.  Perhaps I 
misunderstood?


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/#review40849
---


On Sept. 26, 2013, 12:03 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112947/
 ---
 
 (Updated Sept. 26, 2013, 12:03 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 322768
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 - Interest category disappears -
 Steps to Reproduce:
 1.Open a new Dividend transaction.
 2.Enter an interest category and amount.
 3.Enter a new fee category.
 4.On accepting the new category, the interest category and amount have been 
 cleared.
 One line fix in kmymoney/dialogs/transactioneditor.cpp.
 
 - Fixes for KEditWidget visibility issues.
  When editing an investment transaction, interest or fee edit widgets show or 
 hide incorrectly.  This is a fairly long-standing issue, and there have been 
 attempts to fix by delaying the show() or hide() instructions.  This became 
 more pronounced during work to allow column resizing, and Cristian produced a 
 fix for the root cause.  This fix is included here.
 With the above fix in place, it became necessary to revert the delayed show() 
 and hide() calls, and this has been done.
 Of course, nothing is ever as simple as that, and another couple of issues 
 emerged.  Whether or not an interest or fee amount widget is shown depends on 
 the presence or absence of the associated category's text.  It transpired 
 that if, say, an existing Reinvest transaction was edited to be, say a Buy 
 transaction,  the text from the Reinvest interest category was seen by the 
 new Buy entry and resulted in the interest-amount widget being visible when 
 none should appear.  Similarly, if a transaction with a fee set is edited to 
 be a type with no fee expected, for instance, an Add or RemoveShares, then 
 the fee-amount widget became visible when not needed.
 It was necessary to rework the slotUpdateFeeVisibility() and 
 slotUpdateInterestVisibility() functions to take account of the new 
 transaction type.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/widgets/transactioneditorcontainer.h f77b195 
 
 Diff: http://git.reviewboard.kde.org/r/112947/diff/
 
 
 Testing
 ---
 
 Entering and editing various transaction types to ensure only the appropriate 
 widgets became visible or hidden when required.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/
---

(Updated Oct. 1, 2013, 7:33 p.m.)


Review request for KMymoney.


Changes
---

New patch to resolve outstanding points.


Bugs: 322768
http://bugs.kde.org/show_bug.cgi?id=322768


Repository: kmymoney


Description
---

- Interest category disappears -
Steps to Reproduce:
1.Open a new Dividend transaction.
2.Enter an interest category and amount.
3.Enter a new fee category.
4.On accepting the new category, the interest category and amount have been 
cleared.
One line fix in kmymoney/dialogs/transactioneditor.cpp.

- Fixes for KEditWidget visibility issues.
 When editing an investment transaction, interest or fee edit widgets show or 
hide incorrectly.  This is a fairly long-standing issue, and there have been 
attempts to fix by delaying the show() or hide() instructions.  This became 
more pronounced during work to allow column resizing, and Cristian produced a 
fix for the root cause.  This fix is included here.
With the above fix in place, it became necessary to revert the delayed show() 
and hide() calls, and this has been done.
Of course, nothing is ever as simple as that, and another couple of issues 
emerged.  Whether or not an interest or fee amount widget is shown depends on 
the presence or absence of the associated category's text.  It transpired that 
if, say, an existing Reinvest transaction was edited to be, say a Buy 
transaction,  the text from the Reinvest interest category was seen by the new 
Buy entry and resulted in the interest-amount widget being visible when none 
should appear.  Similarly, if a transaction with a fee set is edited to be a 
type with no fee expected, for instance, an Add or RemoveShares, then the 
fee-amount widget became visible when not needed.
It was necessary to rework the slotUpdateFeeVisibility() and 
slotUpdateInterestVisibility() functions to take account of the new transaction 
type.


Diffs (updated)
-

  kmymoney/dialogs/investactivities.cpp 50f33ed 
  kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
  kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
  kmymoney/dialogs/transactioneditor.cpp cfb0f71 
  kmymoney/widgets/kmymoneydateinput.cpp 856efaa 
  kmymoney/widgets/register.h 50ce759 
  kmymoney/widgets/register.cpp d5dd639 
  kmymoney/widgets/transactioneditorcontainer.h f77b195 
  kmymoney/widgets/transactionform.cpp 642e98f 

Diff: http://git.reviewboard.kde.org/r/112947/diff/


Testing
---

Entering and editing various transaction types to ensure only the appropriate 
widgets became visible or hidden when required.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 464
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192725#file192725line464
 
  Are you sure that this is valid since   categoryId.clear() is 
  called above? This only keeps the data in the widget.
 
 Allan Anderson wrote:
 Yes it is.  As explained above, the data in the widget is the name of the 
 interest category, but we're dealing now with a new fee category.  If we 
 clear this text, the interest category has lost its name and it needs to be 
 added again once the fee category dialog closes.
 It definitely fixes the issue, but whether there is something deeper, I 
 don't know.
 
 Cristian Oneț wrote:
 See my question above, when accepting the transaction what will be the 
 value of 'categoryId' if it has been cleared and only the name was kept? Is 
 it being recomputed based on the name which was kept?
 
 Allan Anderson wrote:
 'categoryId' is empty on entry, in these circumstances - when adding a 
 new category.  Editing an existing one doesn't come here.
 
 Cristian Oneț wrote:
 OK, then it's safe.
 
 Allan Anderson wrote:
 Not pushing, but want to be certain whether that means OK to ship, or 
 just you're OK with that particular point?  Didn't want it to be in limbo.
 Tidied up and retested.  All looks OK with no flicker.

 
 Cristian Oneț wrote:
 I was waiting for the updated patch to fix the other issues we've 
 discussed in this review request to give this patch the 'Ship it!'.
 
 Allan Anderson wrote:
 Sorry to be dim, but which issues do you mean?  If you mean the column 
 width issues, I thought your patch was all you wanted on that.  Perhaps I 
 misunderstood?
 
 Cristian Oneț wrote:
 I've added a comment to signal the issues in this review request which I 
 consider still open.

These points had already been attended to.  Revised patch following


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/#review40849
---


On Sept. 26, 2013, 12:03 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112947/
 ---
 
 (Updated Sept. 26, 2013, 12:03 a.m.)
 
 
 Review request for KMymoney.
 
 
 Bugs: 322768
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Repository: kmymoney
 
 
 Description
 ---
 
 - Interest category disappears -
 Steps to Reproduce:
 1.Open a new Dividend transaction.
 2.Enter an interest category and amount.
 3.Enter a new fee category.
 4.On accepting the new category, the interest category and amount have been 
 cleared.
 One line fix in kmymoney/dialogs/transactioneditor.cpp.
 
 - Fixes for KEditWidget visibility issues.
  When editing an investment transaction, interest or fee edit widgets show or 
 hide incorrectly.  This is a fairly long-standing issue, and there have been 
 attempts to fix by delaying the show() or hide() instructions.  This became 
 more pronounced during work to allow column resizing, and Cristian produced a 
 fix for the root cause.  This fix is included here.
 With the above fix in place, it became necessary to revert the delayed show() 
 and hide() calls, and this has been done.
 Of course, nothing is ever as simple as that, and another couple of issues 
 emerged.  Whether or not an interest or fee amount widget is shown depends on 
 the presence or absence of the associated category's text.  It transpired 
 that if, say, an existing Reinvest transaction was edited to be, say a Buy 
 transaction,  the text from the Reinvest interest category was seen by the 
 new Buy entry and resulted in the interest-amount widget being visible when 
 none should appear.  Similarly, if a transaction with a fee set is edited to 
 be a type with no fee expected, for instance, an Add or RemoveShares, then 
 the fee-amount widget became visible when not needed.
 It was necessary to rework the slotUpdateFeeVisibility() and 
 slotUpdateInterestVisibility() functions to take account of the new 
 transaction type.
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/widgets/transactioneditorcontainer.h f77b195 
 
 Diff: http://git.reviewboard.kde.org/r/112947/diff/
 
 
 Testing
 ---
 
 Entering and editing various transaction types to ensure only the appropriate 
 widgets became visible or hidden when required.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org

Re: [Kmymoney-devel] Review Request 112971: Improve the column sizes auto adjustment of the register

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112971/#review40942
---


Yes, that looks much better, thanks.
Just a minor thing, is that when the window is shrinking, the Details column 
can go all together
before the other columns start to shrink.  However, that issue should go 
if/when my column resizing fix
goes ahead.
I think it would be cheeky of me to say 'Ship it'!

- Allan Anderson


On Sept. 28, 2013, 8:50 a.m., Cristian Oneț wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112971/
 ---
 
 (Updated Sept. 28, 2013, 8:50 a.m.)
 
 
 Review request for KMymoney.
 
 
 Description
 ---
 
 Improve the column sizes auto adjustment of the register when the transaction 
 form is disabled and the register is in read only mode. This improves 
 register column sizes in the Payee, Tags views and in the transaction search 
 and autofill dialogs.
 
 
 This addresses bug 325341.
 http://bugs.kde.org/show_bug.cgi?id=325341
 
 
 Diffs
 -
 
   kmymoney/views/kgloballedgerview.cpp 2057b47 
   kmymoney/widgets/register.h eebe78d 
   kmymoney/widgets/register.cpp 1bdf5bd 
 
 Diff: http://git.reviewboard.kde.org/r/112971/diff/
 
 
 Testing
 ---
 
 Checked the column sizes with the transaction form on/of in the ledger, 
 payees, tags, search and autofill.
 
 
 Thanks,
 
 Cristian Oneț
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investactivities.cpp, line 394
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192722#file192722line394
 
  does the ending '///' mean anything? if this is a placeholder to search 
  for maybe a textual comment would be better suited

Oops. Yes, it's a placeholder, that I use to search for changes I've made.  I 
tend not to use it much nowadays.  I should have removed it, but it was getting 
very late and I slipped up.  There may be one or two others as well.


 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investactivities.cpp, line 701
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192722#file192722line701
 
  Please run astyle-kmymoney.sh before pushing the commit.

I did have it the 'correct' way at first, but noticed that there was 'if (w) 
w-hide()' a few lines above, so changed it because it looked 'wrong' in that 
context.  I'll change them both.  I usually use {} even for a one-liner, but 
don't change existing code.  There are too many.


 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investtransactioneditor.cpp, line 499
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192724#file192724line499
 
  I would go with something like this to make it more readable.
  const bool hideFee = txt.isEmpty() || d-m_activity-type() == 
  MyMoneySplit::AddShares || .. the rest of the activities which don't have a 
  fee
  if (hideFee) {
l-setText();
feeAmount-hide();
  } else {
l-setText(i18n(Fee Amount));
feeAmount-show();
  }

I like that.  I had to add the declaration for label 'l', and add tests for it 
to avoid crashing when not using the form.
Perhaps I should try similar for the interest method.


 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investtransactioneditor.cpp, line 528
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192724#file192724line528
 
  Could this:
  if (cond) {} else if (cond) {}
  be written in a more readable manner? I would write something like:
  if (cond) {} else {}
  since all activity types are either in one or the other codition.

In general, then that is what I would do.  I decided not to do it here because 
it provided immediate information on what was dealt with where, with there 
being so many different activity types.
However, I could make it a comment instead, but I would like to leave the 
information there, for clarity.
Perhaps also remove the existing comment above which doesn't seem to apply any 
more?


 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 462
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192725#file192725line462
 
  If the category id is cleared here then why shouldn't the category 
  editor widget be cleared two lines bellow?

Here, I deliberately left in the '///' as I felt a comment was needed, but I'll 
reposition them slightly.

This was the cause of the problem.  Here, the text was the category name from 
the existing interest category, but we are now here because a fee category has 
just been added. If cleared here, the name of the interest category gets 
removed and has to be reentered.


 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 464
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192725#file192725line464
 
  Are you sure that this is valid since   categoryId.clear() is 
  called above? This only keeps the data in the widget.

Yes it is.  As explained above, the data in the widget is the name of the 
interest category, but we're dealing now with a new fee category.  If we clear 
this text, the interest category has lost its name and it needs to be added 
again once the fee category dialog closes.
It definitely fixes the issue, but whether there is something deeper, I don't 
know.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/#review40849
---


On Sept. 26, 2013, 12:03 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112947/
 ---
 
 (Updated Sept. 26, 2013, 12:03 a.m.)
 
 
 Review request for KMymoney.
 
 
 Description
 ---
 
 - Interest category disappears -
 Steps to Reproduce:
 1.Open a new Dividend transaction.
 2.Enter an interest category and amount.
 3.Enter a new fee category.
 4.On accepting the new category, the interest category and amount have been 
 cleared.
 One line fix in kmymoney/dialogs/transactioneditor.cpp.
 
 - Fixes for KEditWidget visibility issues.
  When editing an investment

Re: [Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

 On Sept. 26, 2013, 10 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.cpp, line 464
  http://git.reviewboard.kde.org/r/112947/diff/1/?file=192725#file192725line464
 
  Are you sure that this is valid since   categoryId.clear() is 
  called above? This only keeps the data in the widget.
 
 Allan Anderson wrote:
 Yes it is.  As explained above, the data in the widget is the name of the 
 interest category, but we're dealing now with a new fee category.  If we 
 clear this text, the interest category has lost its name and it needs to be 
 added again once the fee category dialog closes.
 It definitely fixes the issue, but whether there is something deeper, I 
 don't know.
 
 Cristian Oneț wrote:
 See my question above, when accepting the transaction what will be the 
 value of 'categoryId' if it has been cleared and only the name was kept? Is 
 it being recomputed based on the name which was kept?

'categoryId' is empty on entry, in these circumstances - when adding a new 
category.  Editing an existing one doesn't come here.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/#review40849
---


On Sept. 26, 2013, 12:03 a.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112947/
 ---
 
 (Updated Sept. 26, 2013, 12:03 a.m.)
 
 
 Review request for KMymoney.
 
 
 Description
 ---
 
 - Interest category disappears -
 Steps to Reproduce:
 1.Open a new Dividend transaction.
 2.Enter an interest category and amount.
 3.Enter a new fee category.
 4.On accepting the new category, the interest category and amount have been 
 cleared.
 One line fix in kmymoney/dialogs/transactioneditor.cpp.
 
 - Fixes for KEditWidget visibility issues.
  When editing an investment transaction, interest or fee edit widgets show or 
 hide incorrectly.  This is a fairly long-standing issue, and there have been 
 attempts to fix by delaying the show() or hide() instructions.  This became 
 more pronounced during work to allow column resizing, and Cristian produced a 
 fix for the root cause.  This fix is included here.
 With the above fix in place, it became necessary to revert the delayed show() 
 and hide() calls, and this has been done.
 Of course, nothing is ever as simple as that, and another couple of issues 
 emerged.  Whether or not an interest or fee amount widget is shown depends on 
 the presence or absence of the associated category's text.  It transpired 
 that if, say, an existing Reinvest transaction was edited to be, say a Buy 
 transaction,  the text from the Reinvest interest category was seen by the 
 new Buy entry and resulted in the interest-amount widget being visible when 
 none should appear.  Similarly, if a transaction with a fee set is edited to 
 be a type with no fee expected, for instance, an Add or RemoveShares, then 
 the fee-amount widget became visible when not needed.
 It was necessary to rework the slotUpdateFeeVisibility() and 
 slotUpdateInterestVisibility() functions to take account of the new 
 transaction type.
 
 
 This addresses bug 322768.
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/widgets/transactioneditorcontainer.h f77b195 
 
 Diff: http://git.reviewboard.kde.org/r/112947/diff/
 
 
 Testing
 ---
 
 Entering and editing various transaction types to ensure only the appropriate 
 widgets became visible or hidden when required.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112364: BUG:312816 - Implement resizing of ledger Number column (and others)., and Interest category and amount disappear when new fee entered in Dividend.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112364/
---

(Updated Sept. 25, 2013, 10:48 a.m.)


Status
--

This change has been discarded.


Review request for KMymoney.


Description
---

If I choose to use a complex system for check numbers, such that the whole 
number is not visible,
the only way I have available is to stretch the whole window. However, even 
that doesn't help,
as the whole of the increase is grabbed by the Details column.  I accept that 
it is likely that that
column is going to need to be the widest.  Then, why are the Payment and 
Deposit columns twice the
width of the Balance column, when that column may be likely to have the 
greatest value?  Ditto for
the Date.

This fix allows modification of column widths, but also resizes the individual 
columns to more suitable widths.

I found that Thomas had started to implement something similar some while ago, 
so I have built upon and expanded that.

I found that the edit widgets were particularly troublesome, in failing to 
appear/disappear with the show() and hide()
methods, which I'd previously found when last in this area. Then, when the 
screen was being resized, they flickered
more than acceptable. Eventually, where necessary, I resorted to 
zeroing/resetting the height instead, which resolved
the issue, although with some complication.


This addresses bugs 312816 and 322768.
http://bugs.kde.org/show_bug.cgi?id=312816
http://bugs.kde.org/show_bug.cgi?id=322768


Diffs
-

  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp 39049cf 
  kmymoney/views/kgloballedgerview.cpp 2057b47 
  kmymoney/widgets/register.h eebe78d 
  kmymoney/widgets/register.cpp 1bdf5bd 

Diff: http://git.reviewboard.kde.org/r/112364/diff/


Testing
---

Extensive editing of sample files, and changing back and forth between 
different activity types, which tended to show
problem areas. atype run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112364: BUG:312816 - Implement resizing of ledger Number column (and others)., and Interest category and amount disappear when new fee entered in Dividend.

[Allan Anderson]

 On Sept. 25, 2013, 3:21 a.m., Cristian Oneț wrote:
  I would not ship this patch. I'll elaborate a bit more my comments at 
  https://bugs.kde.org/show_bug.cgi?id=312816#c6 and I'll base my arguments 
  on this recording I made 
  http://kmymoney2.sourceforge.net/screencasts/112364.ogv about how this 
  patch behaves.
  
  1. It breaks with the current behavior of the application (auto resizing 
  columns after data has been entered to fit the entered data)
  2. It introduces bugs, I mean, setting a maxWidth of 9,999,999.00  for 
  the amount fields!? This is exactly what was hurting the number column and 
  the fix should be all about removing a limitation not adding new ones

You've concentrated on forms mode, where there is not much wrong.

1. Depending on content, there was some clipping.
2. There already was in Thomas's implementation a width restriction for the 
number column, so I eased it substantially.

The existing excessive width of some columns can result in clipping elsewhere.  
There are still problems with non/unnecessary appearance of investment widgets.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112364/#review40732
---


On Sept. 15, 2013, 4:26 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112364/
 ---
 
 (Updated Sept. 15, 2013, 4:26 p.m.)
 
 
 Review request for KMymoney.
 
 
 Description
 ---
 
 If I choose to use a complex system for check numbers, such that the whole 
 number is not visible,
 the only way I have available is to stretch the whole window. However, even 
 that doesn't help,
 as the whole of the increase is grabbed by the Details column.  I accept that 
 it is likely that that
 column is going to need to be the widest.  Then, why are the Payment and 
 Deposit columns twice the
 width of the Balance column, when that column may be likely to have the 
 greatest value?  Ditto for
 the Date.
 
 This fix allows modification of column widths, but also resizes the 
 individual columns to more suitable widths.
 
 I found that Thomas had started to implement something similar some while 
 ago, so I have built upon and expanded that.
 
 I found that the edit widgets were particularly troublesome, in failing to 
 appear/disappear with the show() and hide()
 methods, which I'd previously found when last in this area. Then, when the 
 screen was being resized, they flickered
 more than acceptable. Eventually, where necessary, I resorted to 
 zeroing/resetting the height instead, which resolved
 the issue, although with some complication.
 
 
 This addresses bugs 312816 and 322768.
 http://bugs.kde.org/show_bug.cgi?id=312816
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Diffs
 -
 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/views/kgloballedgerview.cpp 2057b47 
   kmymoney/widgets/register.h eebe78d 
   kmymoney/widgets/register.cpp 1bdf5bd 
 
 Diff: http://git.reviewboard.kde.org/r/112364/diff/
 
 
 Testing
 ---
 
 Extensive editing of sample files, and changing back and forth between 
 different activity types, which tended to show
 problem areas. atype run.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 112947: BUG:322768 - Interest category and amount disappear when new fee entered in Dividend. Also, fixes for KEditWidget visibility issues.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112947/
---

Review request for KMymoney.


Description
---

- Interest category disappears -
Steps to Reproduce:
1.Open a new Dividend transaction.
2.Enter an interest category and amount.
3.Enter a new fee category.
4.On accepting the new category, the interest category and amount have been 
cleared.
One line fix in kmymoney/dialogs/transactioneditor.cpp.

- Fixes for KEditWidget visibility issues.
 When editing an investment transaction, interest or fee edit widgets show or 
hide incorrectly.  This is a fairly long-standing issue, and there have been 
attempts to fix by delaying the show() or hide() instructions.  This became 
more pronounced during work to allow column resizing, and Cristian produced a 
fix for the root cause.  This fix is included here.
With the above fix in place, it became necessary to revert the delayed show() 
and hide() calls, and this has been done.
Of course, nothing is ever as simple as that, and another couple of issues 
emerged.  Whether or not an interest or fee amount widget is shown depends on 
the presence or absence of the associated category's text.  It transpired that 
if, say, an existing Reinvest transaction was edited to be, say a Buy 
transaction,  the text from the Reinvest interest category was seen by the new 
Buy entry and resulted in the interest-amount widget being visible when none 
should appear.  Similarly, if a transaction with a fee set is edited to be a 
type with no fee expected, for instance, an Add or RemoveShares, then the 
fee-amount widget became visible when not needed.
It was necessary to rework the slotUpdateFeeVisibility() and 
slotUpdateInterestVisibility() functions to take account of the new transaction 
type.


This addresses bug 322768.
http://bugs.kde.org/show_bug.cgi?id=322768


Diffs
-

  kmymoney/dialogs/investactivities.cpp 50f33ed 
  kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
  kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
  kmymoney/dialogs/transactioneditor.cpp 39049cf 
  kmymoney/widgets/transactioneditorcontainer.h f77b195 

Diff: http://git.reviewboard.kde.org/r/112947/diff/


Testing
---

Entering and editing various transaction types to ensure only the appropriate 
widgets became visible or hidden when required.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112364: BUG:312816 - Implement resizing of ledger Number column (and others)., and Interest category and amount disappear when new fee entered in Dividend.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112364/
---

(Updated Sept. 15, 2013, 4:26 p.m.)


Review request for KMymoney.


Changes
---

As agreed below, this is to concentrate on the column width issues.
It is almost exclusively in the Register class, with revised versions of 
Register::adjustColumn(int col) and Register::resize(int col, bool force), 
which are much the same as in the original submission.
This will ensure each column is wide enough for its data, so no need (or 
ability) for the user to resize any individual columns.  Also, at the moment, 
while entering or editing a transaction, the width of all the edit edit boxes 
is considerably hidden by calculator or date entry buttons.  This is improved, 
and the Number column width can expand while editing, so the user can actually 
see all of his entry, instead of it scrolling out of sight.
The whole window may be resized, as now, but I'm abandoning, for now, the 
long-standing issue of investment edit widgets appearing/disappearing 
illogically.
The overall size of the patch is much reduced.  It is a replacement for the 
original one.


Description
---

If I choose to use a complex system for check numbers, such that the whole 
number is not visible,
the only way I have available is to stretch the whole window. However, even 
that doesn't help,
as the whole of the increase is grabbed by the Details column.  I accept that 
it is likely that that
column is going to need to be the widest.  Then, why are the Payment and 
Deposit columns twice the
width of the Balance column, when that column may be likely to have the 
greatest value?  Ditto for
the Date.

This fix allows modification of column widths, but also resizes the individual 
columns to more suitable widths.

I found that Thomas had started to implement something similar some while ago, 
so I have built upon and expanded that.

I found that the edit widgets were particularly troublesome, in failing to 
appear/disappear with the show() and hide()
methods, which I'd previously found when last in this area. Then, when the 
screen was being resized, they flickered
more than acceptable. Eventually, where necessary, I resorted to 
zeroing/resetting the height instead, which resolved
the issue, although with some complication.


This addresses bugs 312816 and 322768.
http://bugs.kde.org/show_bug.cgi?id=312816
http://bugs.kde.org/show_bug.cgi?id=322768


Diffs (updated)
-

  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp 39049cf 
  kmymoney/views/kgloballedgerview.cpp 2057b47 
  kmymoney/widgets/register.h eebe78d 
  kmymoney/widgets/register.cpp 1bdf5bd 

Diff: http://git.reviewboard.kde.org/r/112364/diff/


Testing
---

Extensive editing of sample files, and changing back and forth between 
different activity types, which tended to show
problem areas. atype run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112364: BUG:312816 - Implement resizing of ledger Number column (and others)., and Interest category and amount disappear when new fee entered in Dividend.

[Allan Anderson]

 On Sept. 10, 2013, 7:20 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/transactioneditor.h, line 155
  http://git.reviewboard.kde.org/r/112364/diff/1/?file=185613#file185613line155
 
  Is making this public a good idea?

I had already reverted it here to protected.


 On Sept. 10, 2013, 7:20 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investactivities.cpp, line 219
  http://git.reviewboard.kde.org/r/112364/diff/1/?file=185610#file185610line219
 
  Is there a reason why the show of shares and price was move out 
  from the for loop?

Within the loop, they were handled as QWidgets and were misbehaving.  I needed 
them to be kMyMoneyEdit, to access its children.


 On Sept. 10, 2013, 7:20 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investactivities.cpp, line 318
  http://git.reviewboard.kde.org/r/112364/diff/1/?file=185610#file185610line318
 
  Same questions as Buy::showWidgets()

Same answer.


On Sept. 10, 2013, 7:20 a.m., Allan Anderson wrote:
  Another nice thing would be to have a separate patch for each individual 
  functionality. Right now we have two things here 1 - ledger resizing and 2 
  - investment editor fix for widgets that are visible when they should not 
  be. For me it's not that easy to make a review this way.
 
 Allan Anderson wrote:
 The two came along together.  I can do this for these changes in the 
 investment editor, but most of the trouble was in the new code, even though 
 the methods were identical or near-identical.  So, for these, there is no 
 patch fixing something, as there was no code there before.
 Anyway, I'll see if I can do some separation.
 
 Allan Anderson wrote:
 Incidentally, this problem with the edit widgets disappearing/appearing 
 when wanted/unwanted is present in an unpatched git HEAD, and has been there 
 for a long while.  Opening any investment transaction, then resizing the 
 window,  will produce it with one or more widgets.
 From some fresh experiments, if a hide() (or show()) does not do the 
 necessary, then often replacing with setCalculatorButtonVisible(false) and 
 lineedit()-hide() does the trick.  Sadly though, that does not work in 
 every case, and it looks like the only sure-fire fix is the one I came up 
 with, setting the widget height to zero instead of hiding.


What I'm working on and proposing is, for now, to concentrate on the column 
width issues.
It is almost exclusively in the Register class, with revised versions of 
Register::adjustColumn(int col) and Register::resize(int col, bool force), 
which are much the same as in the original submission.
This will ensure each column is wide enough for its data, so no need (or 
ability) for the user to resize any individual columns.  Also, at the moment, 
while entering or editing a transaction, the width of all the edit edit boxes 
is considerably hidden by calculator or date entry buttons.  This is improved, 
and the number column width can expand while editing, so the user can actually 
see all of his entry, instead of it scrolling out of sight.
The whole window may be resized, as now, but I'm abandoning the long-standing 
issue of investment edit widgets appearing/disappearing illogically.
The overall size of the patch is much reduced.
I hope you agree for me to proceed with this.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112364/#review39691
---


On Aug. 29, 2013, 5:12 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112364/
 ---
 
 (Updated Aug. 29, 2013, 5:12 p.m.)
 
 
 Review request for KMymoney.
 
 
 Description
 ---
 
 If I choose to use a complex system for check numbers, such that the whole 
 number is not visible,
 the only way I have available is to stretch the whole window. However, even 
 that doesn't help,
 as the whole of the increase is grabbed by the Details column.  I accept that 
 it is likely that that
 column is going to need to be the widest.  Then, why are the Payment and 
 Deposit columns twice the
 width of the Balance column, when that column may be likely to have the 
 greatest value?  Ditto for
 the Date.
 
 This fix allows modification of column widths, but also resizes the 
 individual columns to more suitable widths.
 
 I found that Thomas had started to implement something similar some while 
 ago, so I have built upon and expanded that.
 
 I found that the edit widgets were particularly troublesome, in failing to 
 appear/disappear with the show() and hide()
 methods, which I'd previously found when last in this area. Then, when the 
 screen was being resized, they flickered
 more than acceptable. Eventually, where necessary, I resorted

Re: [Kmymoney-devel] Review Request 112364: BUG:312816 - Implement resizing of ledger Number column (and others)., and Interest category and amount disappear when new fee entered in Dividend.

[Allan Anderson]

 On Sept. 10, 2013, 7:20 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investtransactioneditor.h, line 166
  http://git.reviewboard.kde.org/r/112364/diff/1/?file=185611#file185611line166
 
  Is register really an 'int'?

This is in fact not used.  I suspect the 'int' was a 'helpful guess' from 
KDevelop that I didn't pursue.


On Sept. 10, 2013, 7:20 a.m., Allan Anderson wrote:
  Another nice thing would be to have a separate patch for each individual 
  functionality. Right now we have two things here 1 - ledger resizing and 2 
  - investment editor fix for widgets that are visible when they should not 
  be. For me it's not that easy to make a review this way.
 
 Allan Anderson wrote:
 The two came along together.  I can do this for these changes in the 
 investment editor, but most of the trouble was in the new code, even though 
 the methods were identical or near-identical.  So, for these, there is no 
 patch fixing something, as there was no code there before.
 Anyway, I'll see if I can do some separation.

Incidentally, this problem with the edit widgets disappearing/appearing when 
wanted/unwanted is present in an unpatched git HEAD, and has been there for a 
long while.  Opening any investment transaction, then resizing the window,  
will produce it with one or more widgets.
From some fresh experiments, if a hide() (or show()) does not do the 
necessary, then often replacing with setCalculatorButtonVisible(false) and 
lineedit()-hide() does the trick.  Sadly though, that does not work in every 
case, and it looks like the only sure-fire fix is the one I came up with, 
setting the widget height to zero instead of hiding.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112364/#review39691
---


On Aug. 29, 2013, 5:12 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112364/
 ---
 
 (Updated Aug. 29, 2013, 5:12 p.m.)
 
 
 Review request for KMymoney.
 
 
 Description
 ---
 
 If I choose to use a complex system for check numbers, such that the whole 
 number is not visible,
 the only way I have available is to stretch the whole window. However, even 
 that doesn't help,
 as the whole of the increase is grabbed by the Details column.  I accept that 
 it is likely that that
 column is going to need to be the widest.  Then, why are the Payment and 
 Deposit columns twice the
 width of the Balance column, when that column may be likely to have the 
 greatest value?  Ditto for
 the Date.
 
 This fix allows modification of column widths, but also resizes the 
 individual columns to more suitable widths.
 
 I found that Thomas had started to implement something similar some while 
 ago, so I have built upon and expanded that.
 
 I found that the edit widgets were particularly troublesome, in failing to 
 appear/disappear with the show() and hide()
 methods, which I'd previously found when last in this area. Then, when the 
 screen was being resized, they flickered
 more than acceptable. Eventually, where necessary, I resorted to 
 zeroing/resetting the height instead, which resolved
 the issue, although with some complication.
 
 
 This addresses bugs 312816 and 322768.
 http://bugs.kde.org/show_bug.cgi?id=312816
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/views/kgloballedgerview.h 04a6303 
   kmymoney/views/kgloballedgerview.cpp 78d98b2 
   kmymoney/widgets/register.h eebe78d 
   kmymoney/widgets/register.cpp 1bdf5bd 
   kmymoney/widgets/transactionform.cpp 642e98f 
 
 Diff: http://git.reviewboard.kde.org/r/112364/diff/
 
 
 Testing
 ---
 
 Extensive editing of sample files, and changing back and forth between 
 different activity types, which tended to show
 problem areas. atype run.
 
 
 Thanks,
 
 Allan Anderson
 


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

Re: [Kmymoney-devel] Review Request 112364: BUG:312816 - Implement resizing of ledger Number column (and others)., and Interest category and amount disappear when new fee entered in Dividend.

[Allan Anderson]

 On Sept. 10, 2013, 7:20 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investactivities.cpp, line 198
  http://git.reviewboard.kde.org/r/112364/diff/1/?file=185610#file185610line198
 
  Could you add a little comment on why do you need to set the rowHeight 
  later to this value?

From the last paragraph of the description above -
I found that the edit widgets were particularly troublesome, in failing to 
appear/disappear with the show() and hide()
methods, which I'd previously found when last in this area. Then, when the 
screen was being resized, they flickered
more than acceptable. Eventually, where necessary, I resorted to 
zeroing/resetting the height instead, which resolved
the issue, although with some complication.


 On Sept. 10, 2013, 7:20 a.m., Cristian Oneț wrote:
  kmymoney/dialogs/investactivities.cpp, line 204
  http://git.reviewboard.kde.org/r/112364/diff/1/?file=185610#file185610line204
 
  Also I don't see a reason why the timeouts passed to QTimer::signleShot 
  vary that much, 5, 100, 150, is there a reason for it?

I can't give an explanation for why the should differ.  Having previously 
encountered this same problem some why ago, Thomas advised using the timer.  As 
before, I started off using a small value, but found that this didn't work in 
the new near-identical code area. So, I started to increase the delay.  In come 
cases, I had to use a large delay in order to see that the widget did finally 
respond.  I then started to reduce the value until failure, and then back-off.  
Having got a particular widget finally responding, after further work I found 
that the problem was still sometimes there and had to make further adjustments. 
 In many cases, this 'tuning' was revisited.
When finally all appeared OK, I then found that during actual window resizing, 
some widgets would flicker quite badly during their repositioning, depending 
upon the speed of mouse movement.  The only way I could resolve that was by 
zeroing the widget height instead of hiding it.


On Sept. 10, 2013, 7:20 a.m., Allan Anderson wrote:
  Another nice thing would be to have a separate patch for each individual 
  functionality. Right now we have two things here 1 - ledger resizing and 2 
  - investment editor fix for widgets that are visible when they should not 
  be. For me it's not that easy to make a review this way.

The two came along together.  I can do this for these changes in the investment 
editor, but most of the trouble was in the new code, even though the methods 
were identical or near-identical.  So, for these, there is no patch fixing 
something, as there was no code there before.
Anyway, I'll see if I can do some separation.


- Allan


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112364/#review39691
---


On Aug. 29, 2013, 5:12 p.m., Allan Anderson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/112364/
 ---
 
 (Updated Aug. 29, 2013, 5:12 p.m.)
 
 
 Review request for KMymoney.
 
 
 Description
 ---
 
 If I choose to use a complex system for check numbers, such that the whole 
 number is not visible,
 the only way I have available is to stretch the whole window. However, even 
 that doesn't help,
 as the whole of the increase is grabbed by the Details column.  I accept that 
 it is likely that that
 column is going to need to be the widest.  Then, why are the Payment and 
 Deposit columns twice the
 width of the Balance column, when that column may be likely to have the 
 greatest value?  Ditto for
 the Date.
 
 This fix allows modification of column widths, but also resizes the 
 individual columns to more suitable widths.
 
 I found that Thomas had started to implement something similar some while 
 ago, so I have built upon and expanded that.
 
 I found that the edit widgets were particularly troublesome, in failing to 
 appear/disappear with the show() and hide()
 methods, which I'd previously found when last in this area. Then, when the 
 screen was being resized, they flickered
 more than acceptable. Eventually, where necessary, I resorted to 
 zeroing/resetting the height instead, which resolved
 the issue, although with some complication.
 
 
 This addresses bugs 312816 and 322768.
 http://bugs.kde.org/show_bug.cgi?id=312816
 http://bugs.kde.org/show_bug.cgi?id=322768
 
 
 Diffs
 -
 
   kmymoney/dialogs/investactivities.cpp 50f33ed 
   kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
   kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
   kmymoney/dialogs/transactioneditor.h f07dafb 
   kmymoney/dialogs/transactioneditor.cpp 39049cf 
   kmymoney/views/kgloballedgerview.h 04a6303 
   kmymoney/views

Re: [Kmymoney-devel] An awful thing happened-need advice on charset

[allan Anderson]

If you were working on the XML file, do you not have the .kmy file
anywhere?  No old version anywhere?
 On Sep 5, 2013 12:28 AM, Алина Бадорина badorina_al...@mail.ru wrote:

 Dear Sirs,

 I have to turn for help as a very big problem occured when I studied you
 great program from inside. I tried to make the program to add information
 from my credit card to the XML file and opened the XML file in wrong
 charset and .. !SAVED!. So now I lost all the data that I have been
 inserting for more than 5 years.
 Please advise on the following:
 1. Is there sny oppotunity to recover the data?
 2. What charset should I use to recover the hyroglific file?

 Thank you in advance for your kind help.

 Alina

 ___
 KMyMoney-devel mailing list
 KMyMoney-devel@kde.org
 https://mail.kde.org/mailman/listinfo/kmymoney-devel


___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] Review Request 112364: BUG:312816 - Implement resizing of ledger Number column (and others)., and Interest category and amount disappear when new fee entered in Dividend.

[Allan Anderson]

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112364/
---

Review request for KMymoney.


Description
---

If I choose to use a complex system for check numbers, such that the whole 
number is not visible,
the only way I have available is to stretch the whole window. However, even 
that doesn't help,
as the whole of the increase is grabbed by the Details column.  I accept that 
it is likely that that
column is going to need to be the widest.  Then, why are the Payment and 
Deposit columns twice the
width of the Balance column, when that column may be likely to have the 
greatest value?  Ditto for
the Date.

This fix allows modification of column widths, but also resizes the individual 
columns to more suitable widths.

I found that Thomas had started to implement something similar some while ago, 
so I have built upon and expanded that.

I found that the edit widgets were particularly troublesome, in failing to 
appear/disappear with the show() and hide()
methods, which I'd previously found when last in this area. Then, when the 
screen was being resized, they flickered
more than acceptable. Eventually, where necessary, I resorted to 
zeroing/resetting the height instead, which resolved
the issue, although with some complication.


This addresses bugs 312816 and 322768.
http://bugs.kde.org/show_bug.cgi?id=312816
http://bugs.kde.org/show_bug.cgi?id=322768


Diffs
-

  kmymoney/dialogs/investactivities.cpp 50f33ed 
  kmymoney/dialogs/investtransactioneditor.h 3e62c2a 
  kmymoney/dialogs/investtransactioneditor.cpp e9f87fb 
  kmymoney/dialogs/transactioneditor.h f07dafb 
  kmymoney/dialogs/transactioneditor.cpp 39049cf 
  kmymoney/views/kgloballedgerview.h 04a6303 
  kmymoney/views/kgloballedgerview.cpp 78d98b2 
  kmymoney/widgets/register.h eebe78d 
  kmymoney/widgets/register.cpp 1bdf5bd 
  kmymoney/widgets/transactionform.cpp 642e98f 

Diff: http://git.reviewboard.kde.org/r/112364/diff/


Testing
---

Extensive editing of sample files, and changing back and forth between 
different activity types, which tended to show
problem areas. atype run.


Thanks,

Allan Anderson

___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel