Modern aliases

[tiennou]

Sorry for the name, you know that's a hard problem ;-).

This resurrects the switch to NSURL bookmarks I tried to do (#2075), since I wanted to fix the class of network-alias-stalls bugs we have and the main QS method we had actually doesn't handle all the bells and whistles of bookmark resolution. Maybe I should have added a comment on -[NSFileManager resolveAliasAtPath:] and friends but, well, I didn't. Consider yourselves warned 😏.

This one fixes the issue Rob had with the hundreds of warning messages had because symlinks. Since I wasn't keen on basing the new resolver because of shaky grounds, I preferred to keep the symlink-checking separate.

There's also a fix for -infoRec returning the symlink target info instead of the symlink itself, which makes symlink actually "invisible" (and non-working, it would seem — at least here, current master doesn't allow me to right-arrow into a symlink because the code passes a symlink path to some directory-parsing code). I wonder how you guys feel about that, especially in light of #1897, #1485, and the broken test ;-).

The icing on the cake is that "resolvables" show their target path instead of theirs.

Based on the deprecation cleanup branch for motivation so the diff is cleaner.

[skurfer]

Nice to see you, as always. So, let’s see. Some quick testing I did shows that this fixes

• Relaunch action missing for applications installed by brew cask #2036
• Symlinked applications catalog error (1.2.2) #2030
• occasional hangs  #1977 (I could never reproduce it, but I'm taking your word)
• Can't traverse directory symlink #1952
• Can’t right-arrow into symbolic links #1884
• qs refuses to follow symlinks located in /applications #1498 (if it was ever really broken)

In other words, every known issue with symbolic links. I had them all assigned to me, but hadn’t gotten to them yet. Nice work!

Regarding my comment above about the UTI comparison, I found an old comment that suggests this test might not even work, but I’m seeing the target path in the details, so maybe it was fixed at the OS level?

Personally, I’d like the target path in the details string prefixed with things like “Link to” and “Alias of”. Not saying you need to bother with that right now, though. I can add it later.

[tiennou]

I wasn't too keen on adding the -isSymlink thing, but maybe for consistency that's better. I'll try to review how much manual symlink testing there is, and if it can consolidate some tests, I'll add it here.

#2063 ? I think you meant #2064. #2030 I'm not sure because of Path Finder being involved, but I think that's a red-herring. The others should be fixed, given the few tests I ran.

Speaking of tests, I'll write some. I don't think the ones we currently have cover all the cases so they behave themselves (aliased symlinks anyone ? 😆).

The comment you point out is the error I had, because of the "code passes a symlink path to some directory-parsing code". -fileUTI failing was the same reason, it returned the target's UTI.

I made my details changes while running Bezel, the goal was to make the "resolvable" nature apparent by displaying the target's path instead of the resolvable in that case, which IMHO looks nicer than (symlink) and less jarring that what you're proposing (still a path).

[skurfer]

Actually, I meant #2036. I’ve updated my comment.

the goal was to make the "resolvable" nature apparent by displaying the target's path instead of the resolvable in that case, which IMHO looks nicer than (symlink) and less jarring that what you're proposing (still a path).

It’s apparent to me, but I’m worried it will be confusing to some people. Not something we need to sort out immediately.

[tiennou]

Oh, I'll check that one then. Haven't really looked at what happened to actions on resolvables with that change.

I was hoping that the "little alias arrow" that gets added was sufficient for conveying this was a symlink/alias. Granted, the GUI atm only says "this is a resolvable", not the actual type of it, but I don't think the difference is that meaningful. But yeah, this is something we can change.

[tiennou]

Note that after looking at #1977, I will try to make a few network aliases (I had none of those when I wrote that), and then check what's their behavior in both master and this. Sad thing that you can't use your own MBP's file sharing to test...

[tiennou]

#2036 is not fixed by that. It looks to me like a "feature request" to make process actions work on resolvables.

[skurfer]

I looked through the list of actions for any that could be potentially problematic for symbolic links. All of these worked in a sane way:

• Reveal
• Always Open Type With…
• Tagging
• Move to Trash
• Delete
• Get Info
• Move To…
• Copy To…
• Rename
• Get File URL
• Get Path

These had minor problems.

• Get Location - show the HFS+ path to the target file
• Compress - doesn’t seem to do anything (when run on a link to a folder)
• Spotlight comment - puts comments on the original file
• Open in Terminal - doesn’t appear in the list of actions

Only one of those (Get Location) is provided in Core Support and needs to be dealt with here.

Things to note:

• The ones that aren’t working probably never were, but we’re so close to fixing everything, we might as well talk about them now.
• I only tested symbolic links. Not aliases. We should probably check those, too.

[skurfer]

Tried the same actions with aliases. I wouldn’t say they all worked as expected, but they all worked consistent with Finder, which I’d call success.

I think there’s only one place (in the core application) that we’d use -isSymlink, so it’s not a priority. The only reason I’m not merging this now is I don’t have time to write the massive release notes. 😃

[skurfer]

I was going to fix “Get Location” for symbolic links, then merge, but I’m not going to bother.

1. It’s using kCFURLHFSPathStyle from NSURL, so unless we want to write our own code to make HFS paths, it’s going to return the path of the target.
2. kCFURLHFSPathStyle is deprecated.
3. Who cares?

On that last one, I would be OK with just removing the action completely. One less thing we depend on the ND classes for. 😃

[tiennou]

I may or may not have based that PR against the deprecation-cleanup branch ;-). Sorry about that, I'll resubmit.

[skurfer]

I may or may not have based that PR against the deprecation-cleanup branch ;-).

You did, but that’s already been merged, so it doesn’t hurt anything. This has been merged too, so there’s no need to resubmit. 😃

[tiennou]

Are you sure ? I just glanced at it after coming back from a lengthy weekend-of-music, and the graph made me thing those commits weren't in master at all (only your release notes did), so I opened #2157.

[skurfer]

Oh, crap. You’re right. It went back to the branch it was based on, so it did hurt something. Now we have a mess. I’ll try to clean it up tonight and do another release.