New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Latest Download improvements #1109
Conversation
other small improvements: * set `downloadPath` *before* it gets used in error messages * use a set to ignore certain extensions * use the OS default as a fallback instead of hard-coding `~/Downloads`
You're probably right that it's Safari 6 and not the OS. I checked Time Machine for my Safari 5 prefs thinking we could use the absence of the Somewhat unrelated, but have you noticed that |
Yep. It gets called for things like On 13 September 2012 03:33, Rob McBroom notifications@github.com wrote:
|
That's weird, there's a 3 seconds cache for that normally... Does it happen for every proxy or just the Last Download one ? |
On the last test, it only fired twice. I should have looked at the stack trace to see what was calling it, but I was more interested in the problem at hand. :-) I'll look into it a bit. By the way, I added another commit to this last night, so let me know if there are outstanding issues. |
Still a mystery. It's definitely being called multiple times. Since the cache is stored on I added some NSLogs so my clicking in the debugger wouldn't affect the cache timer. This is the result of calling up Latest Download, Current Web Page, then IP Address one time each.
That's as far as I'm going to take it for now. |
Okay, I put appropriate breakpoints and got it to work. You just need to remove the BTW, I also took some time to check BTW 2, there's a |
OK, I'll try that.
Good point. I was calling
As of the most recent commit, it no longer uses Internet Config at all.
I agree, but for this pull request, I'm just trying to get what was already there to work again. :-)
I'll look into that. It should probably be removed or updated and moved to the |
This should prevent repeated calls to recreate the object.
Two new commits. Let me know what you think. |
I can't find any other calls to As for |
Looks good to me. I think the On an almost completely unrelated note, I'm finding the various checks to resolve QSProxyObject and QSRankedObject intrusive. What do you think of adding a |
OK, but short of resolving it, how do we get it up to date? And if we resolve it, aren't we just reintroducing the same problem? I think 3 seconds is an acceptable age for a proxy. And if a particular object needs to be guaranteed fresh (like Current Time), the object source can implement
I think it does. It calls
I see 6 calls to |
Which was my 3 seconds point ;-). I'd say
Then when you got your hands on a QSObject, you'd call |
It's only going to be 3 seconds if the object source says that's an acceptable lag (implicitly, by not overriding it). If it needs to be less, the option is there, so I really think it's fine how it is. Unless you think the only purpose of the cache is to prevent repeated calls in the interface, and not to generally cache in all circumstances. In that case, I can see your point, but I'm not sure it's worth it. Plus I think this could be problematic for some proxies. For example, "Current Selection" doesn't seem to work unless you get it from the cache. That is, you have to “use” it within 3 seconds of calling it or it loses its contents. If it were reset unconditionally, it would never work.
True, but in almost all cases, you don't really get shown anything unless you arrow into it, and at that point, I think you're dealing with the child object, which shouldn't change. But there are other concerns, as I've said. :-)
Ah, OK. So we could call it blindly all over the place. I agree. We should do it. |
This is mainly to fix #1102, but there are some other small improvements as described in the commits.