Get Path action update

[skurfer]

This could be controversial, so I’m just putting it out there for discussion.

I found myself using this action mostly in support e-mails where you want to refer a user to ~/Library/Application Support/Quicksilver/PlugIns or something like that. It annoyed me that I had to replace /Users/rob with ~ so frequently. The literal path has never once been what I wanted, and almost everything I Get Path on is in my home directory.

So my first thought was to create another action to get the ~ path and also make it an alternate for Get Path, but then I thought “Why?”. The ~ path will work in any context where you might use the literal path (a script or something) and like I said, I never want the other form (but that could be just me).

Note that this change has no effect on paths outside the current user’s home.

[fheckl]

Hmm, why not make this behavior the default and keep the old behavior as alternate action?

[skurfer]

I’m not opposed to that. I assume it will be something like “Get Literal Path” in English. What do you suggest for the German localization?

[fheckl]

The current is "Pfad (POSIX)" so I would suggest "Vollständiger Pfad (POSIX)" which is kind of "Get Full Path" as opposed to the abbreviated ~ path.

[pjrobertson]

Well, just had a quick play seems like Quicksilver doesn't understand the
path '~/Something', so if you return that path in the 1st pane, then
Quicksilver won't be able to do something with that path.

Try typing (in text mode) '~/'
Then try typing '/Users/rob/'

I suppose we could change the string sniffing again to resolve ~/ file
paths.

On 27 May 2011 04:34, skurfer <
reply@reply.github.com>wrote:

This could be controversial, so Im just putting it out there for
discussion.

I found myself using this action mostly in support e-mails where you want
to refer a user to ~/Library/Application Support/Quicksilver/PlugIns or
something like that. It annoyed me that I had to replace /Users/rob with
~ so frequently. The literal path has never once been what I wanted, and
almost everything I Get Path on is in my home directory.

So my first thought was to create another action to get the ~ path and
also make it an alternate for Get Path, but then I thought Why?. The ~
path will work in any context where you might use it (a script or something)
and like I said, I never want the other form (but that could be just me).

Note that this change has no effect on paths outside the current users
home.

Reply to this email directly or view it on GitHub:
#331

[skurfer]

The current is "Pfad (POSIX)" so I would suggest "Vollständiger Pfad (POSIX)" which is kind of "Get Full Path" as opposed to the abbreviated ~ path.

Thanks. I’ll probably go with “Get Absolute Path” for the English. That seems to be the most technically correct. Let me know if that changes your suggestion for German. @pjrobertson, can you give me the French?

Well, just had a quick play seems like Quicksilver doesn't understand the
path ~/Something, so if you return that path in the 1st pane, then
Quicksilver won't be able to do something with that path.

Well, you can do plenty with it as a string (paste, type, Large Type, etc.) but it’s true you can’t do file stuff with it, like Open. I would say “Why get the path from a file object if you just want to use it as a file object in the first place?” but I have personally used the Open action on such a string, so I know there’s a use case. Also, it would be nice it you could type a relative path by hand and use file actions on it. I think we’d just need to modify string handling to check for strings starting with ~ and treat them the same as /. I’ll look into it.

[pjrobertson]

Absolute is 'absolu', so it's a pretty easy one in French :)

'Obtenir le chemin absolu'

Not really sure how it could be shortened, so I hope that's OK.

On the resolving ~/ strings as files, I've done some debugging, and it seems there are two problems
Line 87 of QSObject_StringHandling.m is not correctly standardising the paths in the array
Line 100 of this file is returning NO for files that are standardised....

woops - done your dirty work for you :)

Change line 87 to this, and QS'll now resolve ~/ file paths fine :)

files = (NSMutableArray *)[files arrayByPerformingSelector:@selector(stringByStandardizingPath)];

[pjrobertson]

While you're at it, I'd also suggest changing lines 50+ of NSArray_BLTRExtensions.m to use fast enumeration

for(id *anObject in self)
    results = [anObject preformSelector:aSelector withObject:object]
    ...

Or if you want I can do this along with all the other for i,j loops in one pull request

[skurfer]

Added some commits for the actions. I’m going to work on the string handling now.

Also, arrayByPerformingSelector is exactly the type of thing I was looking for to modify all the paths in the Get Path action so I’ll probably update it to use that.

[skurfer]

OK, so this request has suffered a bit of scope creep, but I think it’s good to go now. I’ve made all the suggested changes.

If anyone wants to provide additional localizations (for descriptions and command strings), post them here and I’ll add them.

[fheckl]

Thanks. I’ll probably go with “Get Absolute Path” for the English. That seems to be the most technically correct. Let me know if that changes your suggestion for German. @pjrobertson, can you give me the French?

OK, that would make it "Absoluter Pfad (POSIX)" in German if you want to change it again.

[pjrobertson]

Couple of things:

Where you've used the arrayByPerformingSelector method, you should cast it to an NSMutableArray to stop the warning. See my post where I suggested changing QSObject_StringHandling.m
Since the method is set to return an NSArray, but you're giving it an NSMutableArray, it just keeps the compiler a little happier

I think you should look at trying to keep the name of the existing action the same (getFilePosixPath) and just add a new action with a new name (so basically revert bb1fc17 )
The reason I say this is because after building with this QS, I now have the 'get absolute path' action enabled (in prefs) and the 'get path' action disabled. This could be confusing for users when they upgrade.
"Hey, where did the get path action go? What's this 'get absolute path' business?"

[skurfer]

I saw that but line 96 sets it to an NSArray, so I thought it would be OK. I didn’t notice the warning since there are always so many. I’ll change it. Should I also change line 96 though?

The reason I say this is because after building with this QS, I now have the 'get absolute path' action enabled (in prefs) and the 'get path' action disabled. This could be confusing for users when they upgrade.

I ran into the same thing but I don’t see a way around it. A path beginning with ~ isn’t the POSIX path, so I don’t think we should label it that way (even internally).

I’d even argue that this is less disruptive to users. Neither action is enabled by default, so if you enabled Get Path at some point, you’ll now have Get Absolute Path enabled and Get Path disabled, but…

1. Get Absolute Path exhibits the same behavior that Get Path used to.
2. You will almost certainly reach the Get Absolute Path action by typing whatever you’re used to for Get Path since it contains the same characters in the same order.

So to the user it appears as a simple rename.

[pjrobertson]

Should I also change line 96 though?

No, since NSMutableArray is a sub class of NSArray and any NSMutableArray will inherit all the mothods. Since the `arrayByPerformingSelector' isn't a method from the NSArray class (we defined it), NSMutableArrays don't inherit it, hence the warning. You basically just need to tell the compiler: yeah, this is also a method for an NSMutableArray as well as an NSArray

(...I think. Makes sense to me - plus the compiler doesn't give a warning later on!)

I’d even argue that this is less disruptive to users...

Yeah alright. I've come round to your way of thinking :)

I'd say cast the NSMutableArray in both the times you've used arrayByPerformingSelector then this'll be good to merge

[skurfer]

I made the suggested change.

[fheckl]

Sorry to come late with this but since the method bodies of getFilePaths and getAbsoluteFilePaths only differ by the first line it may be worthwhile to factor out the common remainder into a separate method. But this can also be done independently from this pull...

[pjrobertson]

Sorry to be a pain Rob, but turns out I was wrong on the casting business :(

Here's what @neurolepsy said:

it would work, but that shouldn't be how it's fixed. arrayByPerformingSelector DOES return an mutable array, but that's an implementation detail - it'd declared to return a non-mutable array, and so that's how the object it returns should be treated.
if you didn't have the code behind arrayByPerformingSelector, then the way you'd fix it is to get the regular array it returns, then do something like array = [[array mutableCopy] autorelease]. but, as we can change the code of arrayByPerformingSelector itself, and as it always returns a mutable array anyway, it'd be better to just change the return type of that method from NSArray * to NSMutableArray *.
and subclasses do inherit methods declared in categories - NSMutableArray will respond to arrayByPerformingSelector:. if you're getting an undeclared selector warning, the header file probably hasn't been included, or it doesn't contain all the correct method prototypes.

Perhaps db4011f should be reverted, then the two arrayByPerformingSelector: methods should be changed to return an NSMutableArray, as neurolepsy suggested.

I'd also seen what @fheckl has said, and thought the same thing. Do you think it's worth changing it in this pull? It'd mean we could 'forget' about this once and for all (hopefully!) once it's done.

[skurfer]

I put the duplicated code into a single method as suggested by @fheckl and changed the return type for the arrayByPerformingSelector: methods as suggested by @neurolepsy. That makes the most sense to me (although the warning still seems to be there.)

[pjrobertson]

(although the warning still seems to be there.)

Don't forget to change the .h file.. ;-)
We're making you work for this one! :P

Almost ready to pull :D

[skurfer]

Don't forget to change the .h file

Duh! Thanks.

[pjrobertson]

Seems good now. If @fheckl has nothing more to say, either of us can merge it :)