[Kmymoney-devel] Review Request: disallow single-space match key when matching payee by list of keys

[Marko Käning]

> On Nov. 16, 2012, 7:15 a.m., Marko Käning wrote:
> > kmymoney/mymoney/mymoneypayee.cpp, line 211
> > <http://git.reviewboard.kde.org/r/107138/diff/2/?file=92965#file92965line211>
> >
> >     I haven't tested this code, but I could imagine that it would be necessary to specify the regexp as "^[^ ]*$" in order to be sure that single or multiply occurring spaces get really excluded - but nothing else.
> >     
> >     "[^ ]" would also match the strings " PAYMENT" and "PAYMENT " which you might not want to exclude.
> >     
> >     (Well, I haven't looked into QRegExp though to check whether my above regexp is correct in this case.)
> 
> Łukasz Maszczyński wrote:
>     Just to make sure we're on the same page: validKeyRegExp regexp shall match only if the payee string is _valid_, so if there's no match - the string shall be considered an _invalid_ payee matchstring.
>     
>     Now, let's see what strings would be considered as (in)valid by both expressions:
>     [^ ] matches the following strings: "PAYEE", " PAYEE", "PAYEE ", "PA YEE", "P"
>     [^ ] does not match the following strings: "  ", " ", ""
>     ^[^ ]*$ matches the following strings: "PAYEE", "PA YEE", "P", ""
>     ^[^ ]*$ does not match the following strings: " PAYEE", "PAYEE ", "  ", " "
>     
>     I believe the "[^ ]" regexp is correct.

Sorry for the noise, I should have not commented on this issue, as I see now. I just didn't grasp what was actually required for the task you wanted to achieve. :)

Herewith I withdraw my remarks.


- Marko


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


On Oct. 30, 2012, 6:46 p.m., Łukasz Maszczyński wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107138/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 6:46 p.m.)
> 
> 
> Review request for KMymoney.
> 
> 
> Description
> -------
> 
> There are three possible options when it comes to payee matching for imported transactions:
> 1. no matching
> 2. by payee name
> 3. by list of keys
> 
> When the 3rd option is selected it's easy to make a mistake and add a single space as the match key - the user is unlikely to realize this, as "space key" is not visible on the list (it's just a space, right?) unless selected. The consequence of this mistake is that nearly all transactions will match the payee (to be precise: all transactions which contain at least one space in the 'payee' field).
> 
> The "new" regular expression in the patch requires payee match key to contain at least a single non-space character.
> 
> This patch changes the algorithm of checking the match key: instead of joining multiple match keys into a single string separated with semicolons and matching the whole string with a regular expression, each match key is checked with a regular expression separaterly (results in a simpler regular expression).
> 
> 
> Diffs
> -----
> 
>   kmymoney/mymoney/mymoneypayee.cpp 93e12c8 
>   kmymoney/mymoney/mymoneypayeetest.h d50d61d 
>   kmymoney/mymoney/mymoneypayeetest.cpp 85ee7fd 
> 
> Diff: http://git.reviewboard.kde.org/r/107138/diff/
> 
> 
> Testing
> -------
> 
> new UT written
> make test
> 
> 
> Thanks,
> 
> Łukasz Maszczyński
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kmymoney-devel/attachments/20121117/44a0f91c/attachment-0001.html>