Re: Review Request 128873: Add configuration dialog for CSV Importer

[Łukasz Wojniłowicz]

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

(Updated Sept. 10, 2016, 11:45 a.m.)


Status
--

This change has been marked as submitted.


Review request for KMymoney.


Changes
---

Submitted with commit e0ec294d57d8ee95371078cd4dc643d0410d by ?ukasz 
Wojni?owicz to branch master.


Repository: kmymoney


Description
---

Although autodetecting features were tested multiple ways, they may
cause troubles on another data sets. Give user the possibility to
disable potential troublemakers.


Diffs
-

  kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
  kmymoney/plugins/csvimport/kcm_csvimport.h PRE-CREATION 
  kmymoney/plugins/csvimport/kcm_csvimport.cpp PRE-CREATION 
  kmymoney/plugins/csvimport/kcm_kmm_csvimport.desktop PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettings.kcfg PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettings.kcfgc PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettingsdecl.ui PRE-CREATION 

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


Testing
---


File Attachments


configuration dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/09/09/50f42b98-e239-465b-bf63-21fdd1a42304__Configuration.png


Thanks,

Łukasz Wojniłowicz

Re: Review Request 128873: Add configuration dialog for CSV Importer

[Łukasz Wojniłowicz]

> On Wrz 10, 2016, 7:52 rano, Thomas Baumgart wrote:
> > kmymoney/plugins/csvimport/CMakeLists.txt, line 59
> > 
> >
> > Please remove leftover whitespace char(s)
> 
> Łukasz Wojniłowicz wrote:
> It's done. Do you see any other issues, that should be corrected before 
> commiting it?
> 
> Thomas Baumgart wrote:
> No, except the one with the size.

I didn't see your first comment. I wanted to include that functionality in next 
commit.
I agree, that it shouldn't be here, because it's overburdening user with 
options, so I delited it.


- Łukasz


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


On Wrz 10, 2016, 10:05 rano, Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128873/
> ---
> 
> (Updated Wrz 10, 2016, 10:05 rano)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Although autodetecting features were tested multiple ways, they may
> cause troubles on another data sets. Give user the possibility to
> disable potential troublemakers.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
>   kmymoney/plugins/csvimport/kcm_csvimport.h PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_csvimport.cpp PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_kmm_csvimport.desktop PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfg PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfgc PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettingsdecl.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128873/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> configuration dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/09/50f42b98-e239-465b-bf63-21fdd1a42304__Configuration.png
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128873: Add configuration dialog for CSV Importer

[Thomas Baumgart]

> On Sept. 10, 2016, 7:52 vorm., Thomas Baumgart wrote:
> > kmymoney/plugins/csvimport/CMakeLists.txt, line 59
> > 
> >
> > Please remove leftover whitespace char(s)
> 
> Łukasz Wojniłowicz wrote:
> It's done. Do you see any other issues, that should be corrected before 
> commiting it?

No, except the one with the size.


- Thomas


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


On Sept. 10, 2016, 10:05 vorm., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128873/
> ---
> 
> (Updated Sept. 10, 2016, 10:05 vorm.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Although autodetecting features were tested multiple ways, they may
> cause troubles on another data sets. Give user the possibility to
> disable potential troublemakers.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
>   kmymoney/plugins/csvimport/kcm_csvimport.h PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_csvimport.cpp PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_kmm_csvimport.desktop PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfg PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfgc PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettingsdecl.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128873/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> configuration dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/09/50f42b98-e239-465b-bf63-21fdd1a42304__Configuration.png
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128873: Add configuration dialog for CSV Importer

[Łukasz Wojniłowicz]

> On Wrz 10, 2016, 7:52 rano, Thomas Baumgart wrote:
> > kmymoney/plugins/csvimport/CMakeLists.txt, line 59
> > 
> >
> > Please remove leftover whitespace char(s)

It's done. Do you see any other issues, that should be corrected before 
commiting it?


- Łukasz


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


On Wrz 10, 2016, 10:05 rano, Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128873/
> ---
> 
> (Updated Wrz 10, 2016, 10:05 rano)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Although autodetecting features were tested multiple ways, they may
> cause troubles on another data sets. Give user the possibility to
> disable potential troublemakers.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
>   kmymoney/plugins/csvimport/kcm_csvimport.h PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_csvimport.cpp PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_kmm_csvimport.desktop PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfg PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfgc PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettingsdecl.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128873/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> configuration dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/09/50f42b98-e239-465b-bf63-21fdd1a42304__Configuration.png
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Re: Review Request 128873: Add configuration dialog for CSV Importer

[Łukasz Wojniłowicz]

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

(Updated Wrz 10, 2016, 10:05 rano)


Review request for KMymoney.


Repository: kmymoney


Description
---

Although autodetecting features were tested multiple ways, they may
cause troubles on another data sets. Give user the possibility to
disable potential troublemakers.


Diffs (updated)
-

  kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
  kmymoney/plugins/csvimport/kcm_csvimport.h PRE-CREATION 
  kmymoney/plugins/csvimport/kcm_csvimport.cpp PRE-CREATION 
  kmymoney/plugins/csvimport/kcm_kmm_csvimport.desktop PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettings.kcfg PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettings.kcfgc PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettingsdecl.ui PRE-CREATION 

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


Testing
---


File Attachments


configuration dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/09/09/50f42b98-e239-465b-bf63-21fdd1a42304__Configuration.png


Thanks,

Łukasz Wojniłowicz

Re: Review Request 128873: Add configuration dialog for CSV Importer

[Thomas Baumgart]

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



Regarding the initial window size: why should that be a setting for the user? 
We can provide it with the application and simply store whatever the user 
changed it to. This way, we gain more functionality (the user can control the 
size and this setting is automatically kept for the next run) by reducing the 
number of options. See QWidget::saveGeometry() for details. Or simply store the 
width and height as it is done in the ctor/dtor of KSplitTransactionDlg().


kmymoney/plugins/csvimport/CMakeLists.txt (line 59)


Please remove leftover whitespace char(s)


- Thomas Baumgart


On Sept. 9, 2016, 6:01 nachm., Łukasz Wojniłowicz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128873/
> ---
> 
> (Updated Sept. 9, 2016, 6:01 nachm.)
> 
> 
> Review request for KMymoney.
> 
> 
> Repository: kmymoney
> 
> 
> Description
> ---
> 
> Although autodetecting features were tested multiple ways, they may
> cause troubles on another data sets. Give user the possibility to
> disable potential troublemakers.
> 
> 
> Diffs
> -
> 
>   kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
>   kmymoney/plugins/csvimport/kcm_csvimport.h PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_csvimport.cpp PRE-CREATION 
>   kmymoney/plugins/csvimport/kcm_kmm_csvimport.desktop PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfg PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettings.kcfgc PRE-CREATION 
>   kmymoney/plugins/csvimport/pluginsettingsdecl.ui PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128873/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> configuration dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/09/50f42b98-e239-465b-bf63-21fdd1a42304__Configuration.png
> 
> 
> Thanks,
> 
> Łukasz Wojniłowicz
> 
>

Review Request 128873: Add configuration dialog for CSV Importer

[Łukasz Wojniłowicz]

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

Review request for KMymoney.


Repository: kmymoney


Description
---

Although autodetecting features were tested multiple ways, they may
cause troubles on another data sets. Give user the possibility to
disable potential troublemakers.


Diffs
-

  kmymoney/plugins/csvimport/CMakeLists.txt 216c79c 
  kmymoney/plugins/csvimport/kcm_csvimport.h PRE-CREATION 
  kmymoney/plugins/csvimport/kcm_csvimport.cpp PRE-CREATION 
  kmymoney/plugins/csvimport/kcm_kmm_csvimport.desktop PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettings.kcfg PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettings.kcfgc PRE-CREATION 
  kmymoney/plugins/csvimport/pluginsettingsdecl.ui PRE-CREATION 

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


Testing
---


File Attachments


configuration dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/09/09/50f42b98-e239-465b-bf63-21fdd1a42304__Configuration.png


Thanks,

Łukasz Wojniłowicz