[Top][All Lists]

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

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

From: Fred Kiefer
Subject: Re: [PATCH] NSSavePanel.m -beginSheetForDirectory::::::
Date: Sun, 29 Feb 2004 01:30:41 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030821

Kazunobu Kuriyama wrote:
Fred Kiefer wrote:

This patch tries to add another workaround on top of all the hacks, that
are already there for model and sheet handling. This doesn't look right
to me, so I suggest another, hopefully cleaner, way to resolve this:

I agree with the view above as a general idea or policy; the existing code
relevant to sheet somewhat looks unnatural, and hence someday it needs to
be overhauled.  I don't think, however, the view and the ones blow are
reasonable verification strong enough to keep the bug alive.

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.

Because I don't have access to the Cocoa's header files, I'm not sure how
this can be done.  The NSRun* enum stands for the state of a panel/sheet,
whereas the NS*Button enum for the state of buttons on it.  Obviously,
they are conceptually/semantically different, and we can't identify
the former with the latter in a natural way.  It would be rather
confusing if we could establish a correspondence between them.  In
addition, I don't think relying on the values of enums is a good
programming style; it definitely constitutes buggy code.

So we are able to undo the hack Nicola
put into the save panel years ago and to use stopModalWithCode: there
again, using the button constant as the code.

Yes.  But I could not do this because of lack of time, and because
I found a fairly simple solution.  Look, the patch I sent consists
of only a few lines of code.  It is completely localized in a single
method and doesn't introduce any formidable dependency.  You can easily
remove it when you obtain a complete solution you think of.  All of all,
it does fix the bug, thus making GNUstep better.  It's at least a good
tentative solution for now.

That way everything will
fall into place again and no further hacks will be needed.
And perhaps somebody will in the future even donate a fully working
sheet processing to GNUstep.

I don't understand why you seemingly try to allow GNUstep to live with
the bug, which is simply fixed by a few lines of code.

It sure is easier to reply to this by actually changing the code, which is what I did. You will see, that I mostly had to remove code that was no longer needed. (There is more, that is now obsolete, we should remove this later on, when the new code is proven to work in all cases) It is surprisingly often the case, that clean code is less lines than hacks, still hack are in general easier to do.

As for the new constant value, I don't have access to any Apple headers. I am just able to use the published Application Kit Reference.

And as I said, there still is no sheet handling code in NSApplication, there is only that nasty workaround there, which I put in place some years ago (Flaged with lots of FIXMEs). Feel welcome to implement this cleanly.

Hope this code works for you

reply via email to

[Prev in Thread] Current Thread [Next in Thread]