[PATCH] Add ADDRESS to check printing.

[Christian Stimming]

Dear Thomas,

just a first feedback:

Thanks for the patch! It is great to see more improvements submitted as actual 
patches. I have three comments on the content of your patch:

- In get_check_address() you terminated the lines with backslashes - is there 
any reason for this? Otherwise the backslashes should probably be removed 
here.

- The word "Blocking" IMHO isn't a good wording for the option of how a check 
is being printed. To me this sounds either as a sort of justification style, 
or preventing something from being printed. Can you find some other wording 
for this option? "Blocking Characters" might already be slightly better, but 
still...

- You had to change a glade file (of course, as this is required for such a 
new feature), which means you immediately ran into the fact that gnucash's 
glade files are still glade-2 and not glade-3. It is clear to the gnucash 
developers that all glade files will have to be converted to glade-3 anyway. 
However, this conversion should if at all possible be a separate commit from 
adding new features. Hence, do you think it is possible to submit two patches 
- one for converting print.glade from glade-2 to glade-3 (but without actually 
changing anything), and then your patch where actually new features are added? 
I'm a bit spoiled in that respect because this sort of patch preparation 
becomes especially easy when you keep the gnucash code in a distributed 
versioning system locally, such as git or bazaar or mercurial (hg)... Just a 
thought, but clearly one final improvement for your patch

Two more suggestions on how to contribute such patches, which make it even 
easier to have your patch integrated into the repository and are probably only 
very little additional work for you:

- I suggest you should add the patch as a bugzilla item, where the patch is 
attached as attachment http://wiki.gnucash.org/wiki/Bugzilla . This is much 
easier for e.g. submitting a new patch and marking the original one as 
obsolete, to make sure no developer accidentally commits the old patch into 
SVN.

- Feel free to include a proposed commit message into your patch submission as 
well. There is no length limit on the length of the SVN commit message, so 
your full discussion below could be copied over there right away. However, the 
"first paragraph" of the commit message should be a short summary, and for the 
developers it is easier if you already write down the message here.

Your code looks very good! I am looking forward to have this committed. 
Myself, I won't have time for SVN commits before the weekend, but someone else 
might go ahead and include it into the SVN. Thanks!

Regards,

Christian

Am Montag, 18. Januar 2010 schrieb Thomas Troesch:
> Hello Devel:
> 
> I have attached my first patch attempt.
> 
> The patch does three things.
> 
> 1.  Adds the ability to print an address on checks.  The Print Check dialog
> is changed to have 5 address lines.
> The first field is filled in with the same data as the PAYEE check item.
>  The other lines are blank.  The check format keys
> have a new check-item type 'ADDRESS'.  It behaves like other text
>  check-item types.  The address fields on the dialog are set
>  'not-sensitive'
> if the current format does not have an ADDRESS check item defined.  The
> Custom Field tab has a new line for the address
> location.
> 
> 2.  Added new check format key - Blocking.  This is to put the blocking
> character feature into the check format file.  It is currently
> only in the preferences dialog in the printing tab, and therefore currently
> applies (or not) to all text fields in the check format.  By
> adding the key Blocking_n to the check format, the blocking characters can
> be applied to text type check-items individually.  Its
> really useful when printing stubs and so on.  (NOTE:  blocking characters
> are printed around a field to protect against alteration
> e.g.  ***$100.00***  vs     $100.00)
> 
> 3. Included a Voucher check template.  This is a full page check with the
> check on top and which has two stubs below.  There
> are other voucher formats ( middle and bottom ) , but the check printing
> functionality would require separate templates for
> the different formats.  Also changed the Quicken/Quickbooks US-Letter check
> format to include the ADDRESS check item and blocking
> around the AMOUNT_NUMBER field.  I made a change to the Quicken-wallet
>  check to add blocking characters around the
> AMOUNT_NUMBER field, and I added a clipping rectangle definition to the
> NOTES and PAYEE fields the were placed in the
> check stub area ( if the fields were too long, they would print into the
> check area ).
> 
> Being new to making a patch, I have no idea if I have attached the correct
> file.
> 
> I could not test all of the code paths because it has compiler directives
> based on GTK level  ( 2.10 ).  I also can not properly test
> any check format other than the voucher check.
> 
> My opinion:
> 
> 1.  It seems that the options in edit->preferences->printing are in the
> wrong place.  The blocking characters feature is
> really needed per check-item, not on all text fields for all checks.  The
> same can be said for the 'print date format' feature.  I think
> it should be an attribute of the DATE check item so the date format, which
> in Canada is required for the date field on the check, does not
> print on all dates ( i.e. stubs ).  This would leave the printing tab in
> preferences empty. (NOTE:  the 'print date format' prints for  example
> MMDDYYYY in 8 pt type underneath the printed date field)
> 
> 2.  It would be very nice to find a way for business objects to be accessed
> to provide complete address information from Customer,
> Vendor, and Employee objects.  On the other hand, looking at some old
> requests, and considering my own sense of things after being a long time QB
> user, it may be best that the base application just support contacts with
> addresses.
> 
> Thomas
>