Re: [evolution-patches] [Contacts] Patch for 62142

[Hans Petter Jansson <hpj ximian com>]

On Wed, 2004-08-11 at 18:59 -0700, Chris Toshok wrote:
> On Wed, 2004-08-11 at 13:13 -0500, Hans Petter Jansson wrote:
> > On Wed, 2004-08-11 at 10:15 -0700, Chris Toshok wrote:

> > > Seems you're ending up with a function that does basically what
> > > eab_editor_prompt_to_save_changes does..

> > I wanted to do that, but the editor would be closed prematurely then -
> > the UI will be destroyed, and the save function needs it in case
> > something goes wrong. So I need to leave it open and pass TRUE for
> > should_close.

> _prompt_to_save_changes doesn't destroy the dialog, though - it leaves
> that up to the caller.

Right, and I need the dialog to close when it's done saving. Therefore,
prompt_to_save_changes () is inadequate, while prompt_to_save_changes ()
immediately followed by a close () breaks.


> > As I mentioned, the existing save/close logic ain't all that hot...

> What specifically is wrong with it?

I don't like the fact that there's a should_close flag being passed to
save () functions in addition to the option of closing the dialog
explicitly. I think you should be able to do:

save ();
close ();

The close () function would check if we are doing an async operation
(like just after kicking off a save), and set the "should_close" flag on
the editor if we are. Async operations can check that flag and really
close when they're finished. close () would then be the only way to
close the dialog, and it would do so when it's ready.

Currently, the should_close flag is being kept in async op contexts,
inaccessible to other calls, which imposes a risk of closing the editor
twice (e.g. user does "save and close" and, during a slow save
operation, closes the editor using the WM). With the should_close flag
stored in the editor, we can observe that we're already closing and do
nothing in that case.

Anyway, it's a minor gripe.

--
Hans Petter