yet more quickfill problem in register, and compile warnings, r13487 (was: Re: Imbalance split added on quickfill)

Christian Stimming stimming at tuhh.de
Sun Mar 5 03:43:59 EST 2006


Hi Chris,

Am Sonntag, 5. März 2006 05:32 schrieb Chris Shoemaker:
> > Thanks, Chris, for reacting very quickly.
>
> <OT> BTW, I jump on regressions ASAP because I've learned that, on a
> short time-scale, the cost of fixing a regression is approximately
> exponential in the time elapsed since regression.  

Sure. That's why I also immediately sent email to gnucash-devel instead of 
wait for someone else to spot and/or fix it.

> > *However*, a new problem occurs when using the quickfill: 
>
> Ok, this should be fixed in r13487.  Changing the amount in the
> transaction line now changes both split amounts.

Hm. Well, the original problem now disappeared, but a new one was introduced. 
SVN is still not ready for 1.9.2.

When entering <tab> in the description field, the correct transaction is 
filled, but a *second* new transaction (or at least something that looks like 
it) is also newly added, which upon closer inspection is the account's split 
of the newly entered transaction. Shouldn't happen.

To reproduce: Switch to single-line view mode ("Basic Ledger"). Enter the 
beginning of another transaction in order to activate quickfill. Enter <tab> 
to accept the quickfill proposal. A second new transaction suddenly appears 
as well.

And: Your changes cause a bunch of compiler warnings, copied below, which need 
to be fixed before 1.9.2, too. The ones with "uninitialized value" are 
actually a real bug, I guess. 

The ones about "assignment used as truth value" are rather annoying - I would 
specifically ask you *not* to use assignments as boolean values in loops. As 
we all know, this is a very common error for beginners in C, and because of 
that reason, everyone who reads this code later will first spend some time 
asking herself "is this really intended or is this rather this well-known 
beginner error"? Just to avoid any confusion here, I would kindly ask you to 
make the assignment and the loop condition separate expressions.

And in general I'm confused as to why these compiler warnings didn't cause an 
error at your place. This is gcc-3.3.5 here, but that shouldn't make a 
difference. Did you configure with --disable-error-on-warning? We would ask 
you *not* to do so, as it is precisely this kind of commit problem that is 
caused by disabling the errors on warning.

> Thanks for the detailed instructions.  In case you haven't realized,
> there are so many different usage patterns for the register that the
> chances that my testing covers all your use cases is, unfortunately,
> rather small.  I've never adopted the practice of editing the amount
> in the transaction line, for example.  I always expand the splits.

Didn't know that. Will keep this in mind when describing steps to reproduce.

By the way, the loading of the data file still seems to be slower by approx. a 
factor of two, compared to the r13444 build as of mid this week.

Christian


Compile warnings:

split-register.c: In function `gnc_trans_split_index':
split-register.c:83: warning: suggest parentheses around assignment used as 
truth value
split-register.c: In function 
`gnc_split_register_empty_current_trans_except_split':
split-register.c:1085: warning: suggest parentheses around assignment used as 
truth value
split-register.c:1073: warning: unused variable `splits'
split-register.c:1074: warning: unused variable `node'

split-register-control.c: In function `gnc_find_split_in_trans_by_memo':
split-register-control.c:463: warning: suggest parentheses around assignment 
used as truth value
split-register-control.c: In function `gnc_split_register_auto_completion':
split-register-control.c:703: warning: suggest parentheses around assignment 
used as truth value
split-register-control.c: In function `gnc_find_split_in_trans_by_memo':
split-register-control.c:460: warning: `i' might be used uninitialized in this 
function
split-register-control.c: In function `gnc_split_register_traverse':
split-register-control.c:698: warning: `i' might be used uninitialized in this 
function                                                                       

split-register-load.c: In function `gnc_split_register_load':
split-register-load.c:418: warning: suggest parentheses around assignment used 
as truth value        
                                                         



More information about the gnucash-devel mailing list