date:20150222

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

2015-02-22 Thread Allan Anderson

> On Feb. 12, 2015, 9:24 p.m., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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.

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

2015-02-22 Thread jswami

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

--- Comment #3 from jsw...@pamho.net ---
I agree with you fully, Jack. Thank you. 

By the way: A better warning message would be "You are about to enter a deposit
assigned to an expense category. Is this what you want to do?"

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

2015-02-22 Thread Jack

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

Jack  changed:

   What|Removed |Added

 CC||ostroffjh@users.sourceforge
   ||.net

--- Comment #2 from Jack  ---
There is no indication that is necessarily an error.  It might be charging a
fee or depositing a refund of a fee.  

Other examples of it is not always an error: if I get a rebate from a purchase,
or a refund for returning a product, I assign it to the expense category I used
for the original purchase.While the memo does often explain the situation,
it is not always the case.

In the meantime, it should be easy to create a pair of reports to find such
transactions/splits.

However, showing a warning does make sense in most cases.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

2015-02-22 Thread jswami

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

--- Comment #1 from jsw...@pamho.net ---
Created attachment 91222
  --> https://bugs.kde.org/attachment.cgi?id=91222&action=edit
Typical error created by allowing an expense to be recorded as a deposit

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

2015-02-22 Thread jswami

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

jsw...@pamho.net changed:

   What|Removed |Added

Summary|Detect deposits entered |Detect deposits assigned to
   |with an expense category,   |an expense category, and
   |and withdrawals entered |withdrawals assigned to an
   |with income categories  |income category

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel

[Kmymoney-devel] [kmymoney4] [Bug 344464] New: Detect deposits entered with an expense category, and withdrawals entered with income categories

2015-02-22 Thread jswami

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

Bug ID: 344464
   Summary: Detect deposits entered with an expense category, and
withdrawals entered with income categories
   Product: kmymoney4
   Version: 4.6.6
  Platform: Ubuntu Packages
OS: Linux
Status: UNCONFIRMED
  Severity: wishlist
  Priority: NOR
 Component: general
  Assignee: kmymoney-devel@kde.org
  Reporter: jsw...@pamho.net

Kmymoney allows users to enter a deposit and assign it to an expense category.
Similarly, users can enter a withdrawal and assign it to an income category.
This lets the user create errors. 

Better: When a user assigns a deposit to an expense category, or a withdrawal
to an income category, the program should issue a warning, such as:

"You are about to assign a deposit to an expense category. Is this what you
want to do?"

Thank you.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
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.

2015-02-22 Thread Christian David



> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > This is more a general annotation:
> > 
> > You are giving the child access to it's parent. This violates the 
> > object-oriented principle of modularity. Thus maintaince is harder and the 
> > risk of bugs is higher.
> 
> Allan Anderson wrote:
> Oh dear!  I've been using this since day one of the plugin, and now over 
> a thousand times, for access from one class to common code in another class.  
> I thought I'd seen this method in use in other places in KMM (which isn't to 
> say it's right.).
> What to do?  I probably need to do some studying.
> Hopefully, it shouldn't delay this patch, as that code predates it.

> Hopefully, it shouldn't delay this patch, as that code predates it.

No, it should not delay the patch! I just wanted to mention it.


> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > This should be done in a more modular fashion (general annotation).
> 
> Allan Anderson wrote:
> Sorry, can you explain.

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.


> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > This part can be removed, the flag is set and removed again. Also you 
> > call show() twice.
> 
> Allan Anderson wrote:
> As above.

Without seeing the code: I think, you should just drop the part where the flag 
is removed again and the second ```show()```.


> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > 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.

Do you mean a second "connect" or "ensure it causes an emit of the signal"?


> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > 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?  
> Certainly, I'm happy to move them.  They too predate this patch.

You can prevent multiple connections with ```connect( sender, signal, receiver, 
slot, Qt::UniqueConnection)```.

"Agreed about moving them, but do we want more code at the moment?" - I agree.


> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > ```if (m_wiz->m_pageIntro->isVisible() || 
> > m_wiz->m_pageLinesDate->isVisible())```
> 
> Allan Anderson wrote:
> Done.  I have been trying to watch for that.

It is nice of you to change this :) I was not expecting you to do so, I just 
wanted to mention it for future development.


> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated patch - 0002-BUG-340656.patch
> > 
> >
> > Usually you want member variables to be private.
> 
> Allan Anderson wrote:
> A relic from my earlier days, I'm afraid.  I'll start to look at these as 
> I get the chance.

Just do it if it has a benefit for you. Such changes usually take a lot of time 
without visible advantages.


> On Feb. 12, 2015, 10:24 nachm., Christian David wrote:
> > File Attachment: Updated pa

Re: [Kmymoney-devel] Fwd: Touchdown: KDiagram (KChart/KGantt) has landed in Extragear/Graphics/Libs

2015-02-22 Thread Cristian Oneț

2015-02-22 13:06 GMT+02:00 Alvaro Soliverez :
> FYI

We are already use KDiagram and dropped our internal copy of KDCharts
in the frameworks branch.

Regards,
Cristian

>
> -- Forwarded message -
> From: Friedrich W. H. Kossebau 
> Date: dom, feb 22, 2015 01:54
> Subject: Touchdown: KDiagram (KChart/KGantt) has landed in
> Extragear/Graphics/Libs
>
>
>
> Hi everyone,
>
> last email in this series and for the record:
>
> as announced here
> http://lists.kde.org/?l=kde-core-devel&m=142456043820542&w=2
> and quickly completed by the KDE sysadmin, KDiagram has made it finally into
> existance as a repo in KDE land, living in
> https://projects.kde.org/projects/extragear/graphics/libs/kdiagram
> Right in time for the begin of the Calligra porting :)
>
> As mentioned in the email to kde-core-devel, a first release will not be
> immediately made, but only after also some feedback from Calligra porting
> has
> been gathered. So might be somewhen in March.
>
> Thanks everyone for your interest and support until here.
>
> Please also consider subscribing to bugs.kde.org for kdiagram bugs, contact
> me
> if you want to be added in the default cclist. KMyMoney devs have already
> found the first crash bug... 8)
>
> Cheers
> Friedrich
>
>
> ___
> 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] Fwd: Touchdown: KDiagram (KChart/KGantt) has landed in Extragear/Graphics/Libs

2015-02-22 Thread Alvaro Soliverez

FYI

-- Forwarded message -
From: Friedrich W. H. Kossebau 
Date: dom, feb 22, 2015 01:54
Subject: Touchdown: KDiagram (KChart/KGantt) has landed in
Extragear/Graphics/Libs

Hi everyone,

last email in this series and for the record:

as announced here
http://lists.kde.org/?l=kde-core-devel&m=142456043820542&w=2
and quickly completed by the KDE sysadmin, KDiagram has made it finally into
existance as a repo in KDE land, living in
https://projects.kde.org/projects/extragear/graphics/libs/kdiagram
Right in time for the begin of the Calligra porting :)

As mentioned in the email to kde-core-devel, a first release will not be
immediately made, but only after also some feedback from Calligra porting
has
been gathered. So might be somewhen in March.

Thanks everyone for your interest and support until here.

Please also consider subscribing to bugs.kde.org for kdiagram bugs, contact
me
if you want to be added in the default cclist. KMyMoney devs have already
found the first crash bug... 8)

Cheers
Friedrich
___
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.

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

[Kmymoney-devel] [kmymoney4] [Bug 344464] Detect deposits assigned to an expense category, and withdrawals assigned to an income category

[Kmymoney-devel] [kmymoney4] [Bug 344464] New: Detect deposits entered with an expense category, and withdrawals entered with income categories

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

Re: [Kmymoney-devel] Fwd: Touchdown: KDiagram (KChart/KGantt) has landed in Extragear/Graphics/Libs

[Kmymoney-devel] Fwd: Touchdown: KDiagram (KChart/KGantt) has landed in Extragear/Graphics/Libs

9 matches

Site Navigation

Mail list logo

Footer information