An example hook has been added to imp to automatically set this 
preference, but will only work if using Horde 3.3.5 (our current CVS 
code) or greater due to an issue that was just fixed with pref hooks.

Changes have been made in CVS for this ticket:

http://cvs.horde.org/diff.php/imp/config/hooks.php.dist?rt=horde&r1=1.3.2.6&r2=1.3.2.7&ty=u
http://cvs.horde.org/diff.php/imp/docs/CHANGES?rt=horde&r1=1.699.2.393&r2=1.699.2.394&ty=u

It might help to have an example hook that automatically sets the pref 
to the user's share.

Ok, i think you are right. But at least,you should modify this 
recomendation from imp/config/prefs.php.dist:



// address book to use for adding addresses

// put $cfgSources array element name in the value field.

// Setting value to localsql would allow you to add contacts to MySQL database

// See turba/config/sources.php for more info

$_prefs['add_source'] = array(

     'value' => '',

//  'value' => 'localsql',

     'locked' => false,

     'shared' => true,

     'type' => 'implicit'

);



At least you should mention that it doesn't work in turba's share mode.

I'm with Jan on this. Personally, I'd rather not have the address book 
icon show if no explicit address book is selected in the prefs either 
by the user or by a default value in prefs.php. The value of 
'localsql' is not expected to work when localsql is defined as a 
shared source, so in that case this value should not be used as a 
default value in prefs.php.

One more thing.



The problem is given only in default pref (when you set value => 
'localsql' in prefs.php). If the user enter to options panel and 
selects a diferent addressbook, that addresbook prevails over the 
default addressbook, because of this condition:



if (!in_array($import_source, $addressbooks)) {



I think the patch make more robust the operation, because you can set 
the add_source pref to an addressbook, but that addressbook cound be 
deleted in the future. In that case i think it would be more desirable 
has the operation done over the default addresbook rather than the 
error.

Yes, but the user doesn't choose any addressbook at all, so the user 
should think that the operation is made over the default addressbook.



This error is given when you click in the icon shown near the address 
when you are viewing a mail from IMP. Yes, you are right and it should 
be a second step to choose the target addressbook, but if you don't 
introduce that second step, the only way to give that functionality is 
to work over the default addressbook.

I don't think it's a good idea to silently add the address to a 
different address book than the user intended.

When add_source pref is set to localsql, when you try to add a contact 
from a message you get this error:



Invalid address book: localsql



I think that the correct behavior is to add that contact to the 
default addressbook.



I attach a patch to modify turba/lib/api.php to achieve that.