When using Move to.../Copy to... set the 3rd pane object to the parent directory of the file to be moved/copied

[pjrobertson]

Does exactly what it says on the tin.

Short of a better place to start, I think the 'current' folder is typically quite near where you want to move things to. I found myself doing this a lot lately, so thought I'd implement it.

It's certainly better than a completely random folder

[skurfer]

By the way, I love this change. I always saw the weirdest folders pop up in the third pane.

[skurfer]

If I try to move something out of ~/Downloads, the parent folder in the third pane has a semi-transparent generic folder icon. I think this is because you’re creating the object from scratch every time. Something like this seems to fix it (and probably runs faster for things already in the catalog).

NSString *currentFolderPath = [[[[dObject splitObjects] lastObject] singleFilePath] stringByDeletingLastPathComponent];
QSObject *currentFolderObject = [QSObject objectWithIdentifier:currentFolderPath];
if (!currentFolderObject) {
    // if it wasn't in the catalog, create it from scratch
    currentFolderObject = [QSObject fileObjectWithPath:currentFolderPath];
}

Also, did you notice if you set a breakpoint in this method, it gets run twice? Completely unrelated to this pull request, but it sounds like a good opportunity for a speed-up somewhere.

[pjrobertson]

:) I like how you're improving on this!

I'll make the changes when I get home. (OK, I'm now home and making the changes. Completely forgot about this - work was too fun! :P)

Also, did you notice if you set a breakpoint in this method, it gets run twice? Completely unrelated to this pull request, but it sounds like a good opportunity for a speed-up somewhere.

I've seen that in hundreds and hundred of places with Quicksilver. If we can stop it, then Quicksilver will definitely be much, much faster!

[pjrobertson]

Take a look and see what you think :)

P.S. - you may have just omitted it in your line note, but a mutableCopy retains the object so we need to release it when we're done :)

[skurfer]

I've seen that in hundreds and hundred of places with Quicksilver. If we can stop it, then Quicksilver will definitely be much, much faster!

Yeah, but usually something runs 3 times (once per pane, I assume) or some multiple of 3. Probably pretty tricky to find and fix those. But this is twice, which makes me think it’s probably just a silly mistake somewhere.

As for how the folder object gets created, I looked at fileObjectWithPath: and it’s already doing a check for existing objects so in theory, my fix should be redundant, but in practice, it seemed to fix the issue with the icon.

[pjrobertson]

my fix should be redundant, but in practice, it seemed to fix the issue with the icon.

Yeah looking at fileObjectWithPath: it looks completely redundant. I'll look into it.
I haven't noticed the 'running twice' problem you're talking about, but I'll look into it

[pjrobertson]

FWIW: I still saw the 'semi' transparent problem with your fix, but it was pretty random. Removing your (redundant) check makes no difference for me.

And... the 'running twice' business is: The method is run however many times you type a letter.

e.g. if you type 'm' for 'Move To...' the method is run once.
If you type 'moveto' the method is run 6 times! Good catch, now to fix... :o

[skurfer]

If you type 'moveto' the method is run 6 times! Good catch, now to fix

Ah, this could be a result of having the “Wait before searching” preferences set to 0.0. I’ll bet if I up that, the method will only have to run once.

[pjrobertson]

OK you have many, many commits to look at now! I thought this was going to be an easy one...

So the fix for the 'running twice' part was to check the actual objects as opposed to the ranked objects (which obviously aren't the same as you alter your search)

The commits also:

• Reverts the check to see if the file object exists, as fileObjectWithPath: does this
• Properly defined some setter/getters that were giving a warning. From what I can see, they're always NSMutableArrays
• Moved the 'disabling catalog' part to run for debug builds only. This will mean we'd have to remove the section on 'Catalog' here. I think we'd gain more from users not reporting problems than we would from users not being able to debug a crash. Who's going to go through altering their .plists from 'YES' to 'NO' anyway?!

[pjrobertson]

OK you have many, many commits to look at now! I thought this was going to be an easy one...

So the fix for the 'running twice' part was to check the actual objects as opposed to the ranked objects (which obviously aren't the same as you alter your search)

The commits also:

• Reverts the check to see if the file object exists, as fileObjectWithPath: does this
• Properly defined some setter/getters that were giving a warning. From what I can see, they're always NSMutableArrays
• Moved the 'disabling catalog' part to run for debug builds only. This will mean we'd have to remove the section on 'Catalog' here. I think we'd gain more from users not reporting problems than we would from users not being able to debug a crash. Who's going to go through altering their .plists from 'YES' to 'NO' anyway?!

[skurfer]

Increasing the time before searching did fix the double call, but your fix is still necessary because as I type, the action isn’t changing and so there should be no need to re-validate. Good catch.

Properly defined some setter/getters that were giving a warning.

Maybe this is pre-mature optimization, but it doesn’t need to be mutable, right? There doesn’t seem to be an immutableCopy method though. :-)

[lovequicksilver]

Great! I got 'Scripts archive iPhoto' in pane 3. On relaunch it became 'Public', and now it's 'Example materials and photos'…

[pjrobertson]

Maybe this is pre-mature optimization, but it doesn’t need to be mutable, right? There doesn’t seem to be an immutableCopy method though. :-)

I guess not, but is it worth us changing it? I think it may be a bit premature considering the optimisation gains would be minimal, but the consequences if anything every does try to alter the array would be catastrophic.

[skurfer]

I guess not, but is it worth us changing it?

Nah, leave it.

[pjrobertson]

Is there anything more that needs to be done on this?

I know the changes here grew to a bit more than just the move to.../copy to... action, so I guess it might take a bit more time to test.

[skurfer]

Is there anything more that needs to be done on this?

No, I’ll probably merge this and a couple of your others this afternoon.

[pjrobertson]

Cool, thanks :)

On 31 January 2012 18:25, Rob McBroom <
reply@reply.github.com

wrote:

Is there anything more that needs to be done on this?

No, Ill probably merge this and a couple of your others this afternoon.

---

Reply to this email directly or view it on GitHub:
#665 (comment)