Re: [PATCH] NSSavePanel.m -beginSheetForDirectory::::::

[Fred Kiefer]

Alexander Malmberg wrote:
> Fred Kiefer wrote:
>
> > We should give the correct Cocoa values to the constants
> > NSRunStoppedResponse, NSRunAbortedResponse and  NSRunContinuesResponse.
> > With these there will no longer be a conflict to the values of
> > NSCancelButton and NSOKButton.
>
>
> Relying on two separate enums having distinct values for their constants
> is wrong unless there is explicit documentation stating that the two
> enums are disjoint wrt values. There isn't currently, afaict, any such
> documentation in GNUstep or OpenStep, and I think it'd be a hack to add
> this for just two specific enums.

At least for Cocoa these enums are fully documented, we just need to do 
the same for GNUstep. And what is the hack, you see here? I just don't 
get the meaning of the sentence.

> Instead, if we want to have useful support for passing arbitrary values
> to -stopModalWithCode:, I think we should do the following (which
> somewhat overlaps changes to the handling of -abortModal discussed
> recently):
>
>
> a. Unify handling of -stopModal... and -abortModal (by making all of
> them equivalent to calling -stopModalWithCode: with the right constant).
> This will require some changes in the way timers are handled in
> NSRunLoop to be really useful, but those changes are also needed for
> other reasons, so I think that's appropriate. I've used those changes
> locally for a fairly long time, and it hasn't broken anything (and it
> has fixed some minor issues). I'll send the patch to the list soon, I
> hope.
>
> This does change the semantics of -abortModal. Currently, it raises an
> exception and never returns. However, this is ugly, causes problems with
> exception handlers between -abortModal and the run loop, and can't be
> extended to work across threads. The new behavior would be just like
> -stopModal's. This is consistent with OpenStep (and, I think Cocoa), but
> not OPENSTEP. I don't think compatibility will be an issue; all uses in
> my collection of GNUstep apps are already written as if -abortModal
> would return.
>
>
> b. Remove all semantic meaning from all NSRun.*Response except
> NSRunContinuesResponse (which needs to be special in -runModalSession:).
> Thus, NSRunStoppedResponse and NSRunAbortedResponse are just convenience
> values for -stopModal and -abortModal. This is actually already the
> case, so it just needs to be documented.
>
>
> c. Give NSRunContinuesResponse a well-defined, documented value. Since
> NSRunContinuesResponse has special semantics, apps must avoid using that
> specific value. This is necessary to make it safe to pass in arbitrary
> values.

I almost fully agree to what you write. But this does not clash with 
anything I did change, it just complement it with regard to -abortModal 
and this had already been discussed before.
The point where I may disagree but I am not that sure, is b). What would 
be the result of -runModalSession: if -stopModalWithCode: gets called 
with NSRunStoppedResponse or NSRunAbortedResponse? If this will still be 
that value, things are fine for me.

> To make it easy to use other enums as codes, we should avoid typical
> enum values, so the current value (-1002) is probably as good as it
> gets. It's arbitrary, but non-arbitrary values have a greater risk of
> colliding with other constants.
>
> The values of the other NSRun.*Response don't matter (they have no
> special meaning). Thus, this amounts to documenting the actual value of
> NSRunContinuesResponse.
>
> (Another reason to keep -1002 is that changing the values of these
> constants breaks binary compatibility. GNUMail and MusicBox users will
> want to recompile or there'll be odd issues with modal windows; this may
> have been the cause of some recent bug reports.)

Oops, this is a point I missed in my original change. Thank you for 
pointing this out.
> Anyway, with these changes, it would be possible for anyone to use any
> enum or set of constants as codes in -stopModalWithCode: as long as they
> stay away from the explicitly documented -1002, which seems like a good
> thing. :) Thoughts?