On Jan 18, 2013, at 13:35, Graham Dennis wrote:
>
>
> On 18/01/2013, at 9:34 PM, Christiaan Hofman <[email protected]> wrote:
>
>>
>> On Jan 18, 2013, at 6:48, Graham Dennis wrote:
>>
>>> Hi,
>>>
>>>>> When BibDesk next asks for the URL for the file, BibDesk resolves the URL
>>>>> from the FSRef instead of the alias. The FSRef now points to the PDF in
>>>>> Dropbox's private cache. (This happens in -[BSDKLinkedAliasFile URL]).
>>>>>
>>>>> My proposed solution is to notice if the URL has moved, and re-resolve
>>>>> the alias in this case. This causes the correct file to be resolved.
>>>>> I've attached a patch which corrects this problem.
>>>>
>>>> I'm not entirely in favor of this, as it seems like a pretty big change
>>>> from existing behavior. Suppose you have a reference with linked file at
>>>> ~/foobar.pdf. With BibDesk running, you can move that file in Finder to
>>>> ~/Documents/foobar2013.pdf, and BibDesk should track it. Suppose you then
>>>> create another file at ~/foobar.pdf. If I recall the BD design correctly,
>>>> the reference should point to ~/Documents/foobar2013.pdf, whereas with
>>>> your patch it sounds like it would point to ~/foobar.pdf?
>>>>
>>> If you just move ~/foobar.pdf to ~/Documents/foobar2013.pdf and return to
>>> BibDesk, then with my patch BibDesk updates the link to point to
>>> ~/Documents/foobar2013.pdf (because the alias resolves to this file).
>>>
>>> If the user moved ~/foobar.pdf *and* created a new file in its place before
>>> switching back to BibDesk, it's not obvious to me which file the user
>>> desires the link to resolve to. However with the current design, different
>>> things happen depending on whether or not BibDesk had resolved ~/foobar.pdf
>>> before you moved it. If you had ~/foobar.pdf resolved in BibDesk to an
>>> FSRef, then move the file and create a new file, and then return to
>>> BibDesk, you'll end up with a link to ~/Documents/foobar2013.pdf. However,
>>> if you didn't have BibDesk running, then when you launch BibDesk at the end
>>> the alias will be resolved to ~/foobar.pdf not ~/Documents/foobar2013.pdf.
>>> My patch makes the behaviour the same in both cases (the latter behaviour).
>>>
>>>> Part of the point of using FSRefs is that you avoid mucking about with
>>>> fragile path comparisons, so that makes me a bit nervous too. In fact,
>>>> -[NSURL isEqual:] will actually fail (or at least it used to) by returning
>>>> NO for the same URL returned from different APIs.
>>>>
>>> I wasn't aware of this. I think it's OK in this situation because lastURL
>>> is simply the last value of aURL, and hence both are obtained from
>>> CFURLCreateFromFSRef().
>>>
>>> The full (patched) method implementation looks like:
>>> - (NSURL *)URL;
>>> {
>>> BOOL hadFileRef = fileRef != NULL;
>>> CFURLRef aURL = (hadFileRef || [self fileRef]) ?
>>> CFURLCreateFromFSRef(NULL, fileRef) : NULL;
>>>
>>> BOOL moved = [(NSURL *)aURL isEqual:lastURL] == NO && (aURL != NULL &&
>>> lastURL != nil);
>>> if ((aURL == NULL || moved) && hadFileRef) {
>>> // fileRef was invalid, or URL moved, try to update it
>>> [self setFileRef:NULL];
>>> if ([self fileRef] != NULL)
>>> aURL = CFURLCreateFromFSRef(NULL, fileRef);
>>> }
>>> BOOL changed = [(NSURL *)aURL isEqual:lastURL] == NO && (aURL != NULL
>>> || lastURL != nil);
>>> if (changed) {
>>> [lastURL release];
>>> lastURL = [(NSURL *)aURL retain];
>>> if (isInitial == NO)
>>> [delegate performSelector:@selector(linkedFileURLChanged:)
>>> withObject:self afterDelay:0.0];
>>> }
>>> isInitial = NO;
>>> return [(NSURL *)aURL autorelease];
>>> }
>>>
>>> Note that -[NSURL isEqual:] was previously used by this method in the BOOL
>>> changed = ... line, which I haven't modified.
>>>
>>
>> But that one is completely irrelevant for following the file. It is only
>> used for forcing a UI update. So it's no problem if it returns NO too often.
>>
>>>> If the idea is to add a workaround specifically for Dropbox, do they
>>>> guarantee anything about the location? Will it always be under
>>>> ~/Dropbox/.dropbox.cache? Hitting the filesystem or comparing URLs is
>>>> pretty slow, though...your way might be the only one, but it would be nice
>>>> to understand the implications.
>>>
>>> The idea is a workaround specifically for Dropbox, but I wanted to avoid
>>> relying on the current behaviour of Dropbox. They don't guarantee that the
>>> cache will be in path_to_dropbox/.dropbox.cache, and certainly your dropbox
>>> can be located in a different location from ~/Dropbox.
>>>
>>> As for hitting the filesystem, we're already doing that in
>>> CFURLCreateFromFSRef, and the addition filesystem hit will be limited to
>>> the infrequent occasions when the file has moved.
>>>
>>> Let me know if you have any concerns or suggestions.
>>>
>>> Regards,
>>> Graham
>>
>> Basically you're saying to drop the FSRef, because you always compare the
>> FSRef with the URL and forget about the FSRef whenever they disagree. So why
>> even bother keeping the FSRef? But we do use the FSRef for good reasons.
>> It's much more efficient at following the file object. This leads to extra
>> complexity and fragility. Note that the lastURL is not used for following
>> the linked file at all. It is only used as a way to check whether things
>> have changed in order to update the UI. All logic about following the file
>> is actually in the fileRef method, not in this one. This one only translates
>> that resolution to a final URL. What you do is divide it over two methods,
>> which makes it less transparent and more fragile, and moreover leads to
>> more inconsistentcies. Note that when things change, you don't update the
>> underlying information (unlike what happens in the fileRef method). So your
>> saved data will be inconsistent with the displayed data. So, no, we cannot
>> use this.
>
> You're right, my patch does make the FSRef redundant. I could create a new
> patch to fix these issues, but first I'd like to have a discussion about
> whether the current behaviour is considered correct.
That's not a bug. It's a feature. My point is that you say we should remove a
feature, which is something we don't do.
>
> Apart from my problem that the current behaviour leads to problems
> specifically with Dropbox, which file the link is resolved to depends upon
> whether BibDesk is running and has previously resolved a link when the user
> moves the file and creates a new one.
>
> So, for the situation proposed earlier where file ~/foobar.pdf is moved to
> ~/Documents/foobar2013.pdf and a new ~/foobar.pdf is created, what should
> happen if
> 1) BibDesk was open before changes, and ~/foobar.pdf had been resolved into
> an FSRef and
> 2) if BibDesk was not open during the changes?
>
> My opinion is that the behaviour in both cases should be the same to avoid
> user confusion. Do you agree / disagree?
>
> Thanks,
> Graham
It is not possible to have the same behavior in all cases. So this point is
basically mute. (My point is *not* that it cannot behave the same way in this
particular situation, but more generally.) The point is that if you fix it for
some specific case, it will go wrong for some other case.
The main point is that one cannot distinguish between Dropbox moving a file,
and the user moving a file.
All of this has been discussed in the past. I still see no reason to revisit
it. Your patch does not change that, quite to the contrary, it reinforces our
stand.
Christiaan
------------------------------------------------------------------------------
Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
much more. Get web development skills now with LearnDevNow -
350+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122812
_______________________________________________
Bibdesk-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bibdesk-develop
