Re: GnomeCard, AddressBook fixes.



I've checked and tested the patch.
It looks good for me.

I've just two notes :

1. you do something like:

if (field1_defined && field2_defined && field3_defined)
	save 3 fields
if (field1_defined && field2_defined)
	save 2 fields
if (field1_defined)
	save 1 field

So it should be impossible to save a card with only a first name (I mean
only field2).
Would it be better to do something like:
 
if (field3_defined)
	save 3 fields
if (field2_defined)
	save 2 fields
if (field1_defined)
	save 1 field

With empty filed when not defined.
This will still fix your point 3 'tailing semicolon'.

2. Ali what's your middle name ?
Now that I'm middle name aware (-;

Christophe

Le 2001.08.15 12:15:13 +0200, Ali Akcaagac a écrit :
> hello,
> 
> during my exercise through the balsa code, i've detected
> something strange that should really get fixed as soon
> as possible so here the patch.
> 
> now some serious facts:
> 
> [point 1]-----------------------------------------------
> 
> the 'store to addressbook' and some of the vcard options
> in the code are broken for a longer period now and i took
> the time to fix all of them.
> 
> if you click on an email and want to store the address, the
> organisation field has no function, the data inside it doesn't
> get transmitted to the address structure and never gets saved
> in either gnomecard, ldap or whatever. with other words it was
> simply not present.
> 
> affected file...: src/store-address.c
> status..........: function implemented correctly by me
>                 : now organisation has a reason and work.
> 
> [point 2]-----------------------------------------------
> 
> saving gnomecard files are borked, since the 'if' clausels
> in the gnomecard addressbook file are worthless, they save
> everything, no matter if data is present for the fields or
> not. this ends up in saving uninitialized data to the card.
> 
> example, i mentioned in [point 1], that the organization
> field wasn't present, but the if clausel doesn't give a
> correct status back, so if fields, like lastname, firstname,
> organisation or fullname are empty, then things like the
> namefield and organisation SIGNS are getting saved with empty
> values, this look like this. the if clausels shouldn't check
> for TRUE or FALSE, they should check if there's content in
> the pointers, if not then don't save unused flags. even the
> gnome-pim application doesn't do such things.
> 
> N:;;
> ORG:
> 
> affected file...: libbalsa/address-book-vcard.c
> status..........: function implemented correctly by me
>                 : if clausels looks if the lenght is > 0
>                 : so it know if theres info inside or not.
> 
> [point 3]-----------------------------------------------
> 
> the namefield sometimes gives crappy values back when saved
> in the gnomecard file. sometimes you get things like:
> 
> N:helva;
>        ^----- this semicolon shouldn't be there if we have only
>               the last name and nothing else. another example
>               would look like this.
> 
> N:;mallorca;
>            ^- same thing for this semicolon, even gnome-pim
>               doesn't create them, if the middle name isn't
>               needed as in this case. but balsa's function
>               didn't paid attention for this and stores them.
>               gnome-pim works fine afterwards but for the
>               sake of consistency, this shouldn't happen.
>               the NAME (N:) flag in vcard has this structure
> 
>               N:rummennige;karl;heinz
>                 ---------- ---- -----
>                  ^          ^    ^-------| middlename
>                  |          |
>                  |           ------------| firstname
>                  |
>                   -----------------------| lastname
> 
> affected file...: libbalsa/address-book-vcard.c
> status..........: function implemented correctly by me
>                 : if clausels looks if the lenght is > 0
>                 : so it know if theres info inside or not.
>                 : same issue as in [point 2] wrong if clause.
> 
> [point 4]-----------------------------------------------
> 
> as i mentioned above, middlename. i was really courious,
> why there was no middlename field in the 'store address'
> entry. this is a common vcard implementation and should
> also be present in balsa, since i call it an importand thing
> i often store information about people so balsa cooperates
> correctly with gnome-pim here on my machine and i was really
> unhappy, because balsa didn't paid attention to middlenames.
> saving a name incorrectly is sometimes really sad because
> you miss half of the name.
> 
> e.g.:
> 
> John Merryweather Cooper <jmcoopr@webmail.bmi.net>
> Jacob Ilsø Christensen <firelake@mail1.stofanet.dk>
> Christian J. Robinson <infynity@onewest.net>
> Marco Pesenti Gritti <marco@it.gnome.org>
> 
> i don't want to have names saved that sound like John Cooper,
> Jacob Christensen, Christian Robinson and Marco Gritti. so i
> allowed myself to correct this unbehave, so you don't get a
> normal bugfix, you get something more for free :) this middle
> field got implemented correctly into balsa and operates nicely.
> i only implemented it into gnomecard code, since i don't use
> ldap or ldif so i can't test. i needed to expand the address
> structure within libbalsa. please don't worry if you can't use
> the middle field for ldap or ldif thingy yet, this doesn't change
> anything for you, you can use ldap as you used before without
> noticing anything, the structure enchancement doesn't hurt the
> operationality at all. if someone would be so kind enchancing
> the 'middle_name' field in ldap or ldif, then i would be happy
> to get reports about this, i am also not sure if ldap or ldif
> supports middle name and if it's a valid entry for them. well
> for vcard it is. but as i said no matter what case, it operates
> as it did before. the middle name function is intelligent, since
> the middle name could be different.
> 
> e.g.
> 
> John Merryweather Cooper
>      |----------|
>        middle
> 
> or this
> 
> Klaus van den Hammern
>       |-----|
>        middle
> 
> so if your middle name has more than one 'middle' word it
> extends them automatically.
> 
> affected file...: libbalsa/address-book-vcard.c
>                 : libbalsa/address.c
>                 : libbalsa/address.h
>                 : src/store-address.c
> status..........: function implemented correctly by me
>                 : function was heavily tested with all kind
>                 : of addresses, without any problems.
> 
> [statement]---------------------------------------------
> 
> i have also nailed down some minor other things in the code, that
> are not worth to be mentioned. simply look at the patch to get an
> idea. please patch against latest CVS last changed 14th-aug-2001.
> please note, that this is a serious patch and should be committed.
> as sooner the better, since i loop with some other patches and my
> internal not yet committed patches are getting longer and longer.
> it's better to include them so i can concentrate on other needed
> fixes.
> 
> Ali
> 
> -- 
> Name....: Ali Akcaagac
> Status..: Student Of Computer & Economic Science
> E-Mail..: mailto:ali.akcaagac@stud.fh-wilhelmshaven.de
> WWW.....: http://www.fh-wilhelmshaven.de/~akcaagaa
> 
-- 
Christophe Barbé <christophe.barbe@online.fr>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]