On Wednesday, December 12, 2007, at 10:19AM, "Christiaan Hofman" <[EMAIL 
PROTECTED]> wrote:
>
>On 12 Dec 2007, at 6:17 PM, Adam R. Maxwell wrote:
>
>>
>> On Dec 12, 2007, at 2:59 AM, Christiaan Hofman wrote:
>>
>>>
>>>
>>> On Dec 12, 2007 6:38 AM, Adam R. Maxwell <[EMAIL PROTECTED]> wrote:
>>>
>>> On Dec 11, 2007, at 12:39 PM, Christiaan Hofman wrote:
>>>
>>>>
>>>> On 11 Dec 2007, at 9:30 PM, Adam R. Maxwell wrote:
>>>>
>>>>> Is there any way we can get rid of the finalize changes stuff or
>>>>> endEditingFor:?  I think we're bypassing NSDocument and the
>>>>> formatters in some cases, because of legacy behavior.  I'm thinking
>>>>> of -[BibEditor windowShouldClose:] in particular, but
>>>>> forceEndEditing is another tricky part to handle correctly.
>>>>>
>>>>
>>>> I don't think so. Without binding, I think explicitly ending editing
>>>> is the only way to commit. Why do you say we're bypassing
>>> NSDocument?
>>>
>>> Not necessarily NSDocument, but when you save, it posts a  
>>> notification
>>> that calls finalizeChanges: on open editors, right?  That does a  
>>> brute
>>> force end editing, and the document is saved in spite of any  
>>> alerts in
>>> the editor; I think we should fail to save.  It seems preferable  
>>> to do
>>> something conceptually like this in the document:
>>>
>>> - (BOOL)canSaveDocument{
>>> NSWindowController *wc;
>>> NSArray *editors = [self windowControllers];
>>> for(wc in editors) {
>>>   if ([wc isBibEditor] && [wc didValidate] == NO)
>>>        return NO;
>>> }
>>> return YES;
>>> }
>>>
>>> - (IBAction)saveDocument:(id)sender
>>> {
>>> if ([self canSaveDocument])
>>> [super saveDocument:sender];
>>> else NSBeep()
>>> }
>>>
>>> That would probably be better, if it could work (see below).
>>>
>>>
>>> where didValidate would simply check [[self window]
>>> makeFirstResponder:nil].  That should cause any formatters to fire  
>>> and
>>> check their contents, with no forceEndEditing.  Same thing in
>>> windowShouldClose; it should return NO if [[self window]
>>> makeFirstResponder:nil] fails, not if it has an attached sheet as a
>>> side effect of the finalize call.
>>>
>>> There's likely stuff I'm forgetting or just don't see, but it seems
>>> like that could simplify the validation code at least.  I can't  
>>> figure
>>> out editing; we have a mix of NSControl/NSText/NSFormatter delegates,
>>> and also the recordChangingField:toValue: and
>>> userChangedField:from:to: stuff...it's not clear which is used for
>>> what.
>>>
>>> --
>>> adam
>>>
>>> I also don't like all the different validations. Note that
>>> recordChangingField:... and userChangedField:... have nothing to do
>>> with validation, they are just wrappers around changing a value and
>>> whatever needs to be done (like autogeneration and scripthooks).
>>
>> Yeah, I guessed as much, but recordChangingField was only called from
>> on place, and other code was doing the setting separately.  That was a
>> separate confusion that slipped in while I was typing that message and
>> reading the code :).
>>
>
>I just changed that, it's now called from some other places as well  
>(drag/drop support). I think it used to be called also from the drag/ 
>drop support of the form, at least at some point.
>
>>> But I don't think we can put the validation in one place. Some
>>> validation happens only when the value is committed, as it cannot be
>>> handled by the formatter (e.g. the crossref chain problem). Although
>>> that could perhaps go into control:textShouldEndEditing:.
>>
>> That's a good point.  I realized that eventually when trying to figure
>> out why the citekey was validated twice.
>>
>> I wonder if we need (all) of those formatter delegate methods?
>> There's some modal dialog hell going on there.  The complex string
>> formatter can't enforce balanced braces, of course, so it can't do the
>> partialString validation.  The cite key field can (and now does), so
>> maybe those alerts don't need to be shown?
>
>I'm not sure which alert you're alluding to here?

there's duplication now: 
control:didFailToValidatePartialString:errorDescription: and 
control:didFailToFormatString:errorDescription: may both be used.  the citekey 
field picks up all 3 of those, I think, which is confusing to read (partly due 
to fragileCiteKeyCharacterSet check).

>
>> I recall thinking it was
>> best to show alerts at one point, but as a user I get annoyed at them.
>>
>
>I think that when we revert changes or prevent committing we need to  
>show an alert.
>
>>> What about a window close when the app quits? That does not use
>>> windowShouldClose.
>>
>> Well, the document could refuse to allow application termination via
>> applicationShouldTerminate: if the normal validation path fails.
>>
>
>I'm not sure about the different notifications that go on and the  
>order of them. Also note that when the app terminates windows are  
>closed without going through notifications.

applicationShouldTerminate: is specifically for documents to refuse or delay 
termination, so the document could make the validation checks or even call the 
window close delegate method (NSApplicationTerminateReply).

>Also when the fields are changed they have to stop editing, otherwise  
>the UI is in an inconsistent state wrt to the data.

You mean the typeInfoChange: notification?  It's not going to be inconsistent 
wrt the BibItem, because the BibItem doesn't change.  It'll be out of sync with 
the prefs, in that it won't be displaying some field the user just added, but 
that sounds like an edge case.  It requires you to have an invalid, uncommitted 
edit, and make a change in the prefs.  Closing the editor and reopening it 
would take care of that.  A more common case might be invalid, uncommitted edit 
and changing the type popup; in that case, the popup change should be refused 
until the problem is fixed.

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
Bibdesk-develop mailing list
Bibdesk-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bibdesk-develop

Reply via email to