On 12 Dec 2007, at 8:30 PM, Adam R. Maxwell wrote:
>
> 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).
>
But they happen at different times (one during edit, the other at
commit). Though I think we could remove the
control:didFailToValidatePartialString:errorDescription: dialog, and
just delete the offending characters. I think we should display the
control:didFailToFormatString:errorDescription: dialog, as it gives
the user a choice. Also as the 3rd one resets the cite key in case of
a crossref chain, the user should be warned about the reason (which
may not be obvious at all).
>>
>>> 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).
>
I'm also thinking about the check for unsaved documents, which comes
at another time. It's really important that we take every possibility
into account.
>> 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.
Not just typeinfo change. It could happen through an add/del field,
AppelScript, undo,... Moreover, the changes should be displayed. if
we simply stop updating the table, there is no notification anymore.
And we cannot refuse a change in the type, as we only notice that it
gives a problem after the change has already been made.
Christiaan
-------------------------------------------------------------------------
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
