protocool / AckMate

TextMate plugin (Cocoa) shell for running 'ack'
MIT License
723 stars 34 forks source link

Searching inside symlinked folders opens a new window #5

Open buchi opened 14 years ago

buchi commented 14 years ago

When searching inside a project with symlinked folders, the search result is opened in a new window instead of a tab. Jumping to the correct line doesn't work neither.

protocool commented 14 years ago

http://wiki.github.com/protocool/AckMate/why-didnt-my-file-open-in-a-tab

buchi commented 14 years ago

... but the file is in the project drawer :-)

here are the steps to reproduce the problem: $ mkdir folder1 $ echo "foo" >folder1/file.txt $ mkdir folder2 $ ln -s ../folder1 folder2/symlink $ mate folder2

now search for "foo" with ackmate and click in the result listing

caius commented 14 years ago

At a guess textmate is being given the actual (non-symlinked) path to the file. So as far as TM is concerned the file isn't in the project drawer and opens it in a new window.

stepheneb commented 14 years ago

I've just hit the same issue.

I've got a directory with git clones of many external gems.

I make separate directories with symbolic links to subsets of the gems in the main gems dir to make it easier to load related gems into a textmate project

For example I've created a dir called 'em' that has a collection of gems that related to event-machine:

[em]$ ls -l
amqp-git -> ../gems/amqp-git/
async-rack-git -> ../gems/async-rack-git
brb-git -> ../gems/brb-git
em-http-request-git -> ../gems/em-http-request-git
em-mysql-git -> ../gems/em-mysql-git
em-proxy-git -> ../gems/em-proxy-git
em-synchrony-git -> ../gems/em-synchrony-git
em-websocket-git -> ../gems/em-websocket-git
eventmachine-git -> ../gems/eventmachine-git
faye-git -> ../gems/faye-git
rainbows-git -> ../gems/rainbows-git
thin-git -> ../gems/thin-git

I open all the files in his directory:

[em]$ mate .

At this point if I use textmates project find command to find all instances of "ProxyServer" clicking on a found line will open the file in the current textmate project window.

However if I use ackmates find command clicking on a found line opens the document in a separate window.

Here's a 2m screencast showing the bug: http://screencast.com/t/YTgwOTFhMWY

I haven't done much with obj-C but was looking at the ackmate source to see where this function was performed. Is this done in openProjectFile in JPAckWindowController.m?

I was wondering if there was a way to use a debugger for looking at this. When I build a debug version the run script in the target copies the target build to my TM plugins dir. I had turned on a breakpoint on line 330 in JPAckWindowController.m but no debugger appeared when I ran the ackmate project find command (I had TM closed when I ran the build command and opened it after the build finished).

stepheneb commented 14 years ago

I figured out how to add TextMate as a custom executable: "Project", "New Custom Executable..." and modified the target run script to create a symbolic link if debugging or just install the plugin if building a release:

mkdir -p "$HOME/Library/Application Support/TextMate/PlugIns"
# adapted from: http://developer.apple.com/mac/library/qa/qa2006/qa1500.html
# Depending on the build configuration, either copy or link to the most recent product
rm -f "$HOME/Library/Application Support/TextMate/PlugIns/${FULL_PRODUCT_NAME}"
if [ "${CONFIGURATION}" == "Debug" ]; then
  # if we're debugging, add a symbolic link to the plug-in
  ln -sf "${TARGET_BUILD_DIR}/${FULL_PRODUCT_NAME}" \
    "$HOME/Library/Application Support/TextMate/PlugIns/${FULL_PRODUCT_NAME}"
elif [ "${CONFIGURATION}" == "Release" ]; then
  # if we're compiling for release, just copy the plugin to the Internet Plug-ins folder
  cp -pR "${TARGET_BUILD_DIR}/${FULL_PRODUCT_NAME}" "$HOME/Library/Application Support/TextMate/PlugIns"
fi

I then added a breakpoint at JPAckProcess.m:89

[self.ackTask setArguments:args];

To see what the command line invocation of your emebedded ack looked like.

I could get this to work:

perl ~/dev/macosapps/ackmate-git/build/Debug/AckMate.tmplugin/Contents/Resources/ackmate_ack --follow --nosmart-case --match Proxy

But if I added more of the args you used it didn't work:

$ perl ~/dev/macosapps/ackmate-git/build/Debug/AckMate.tmplugin/Contents/Resources/ackmate_ack --ackmate --follow --nosmart-case --match Proxy
ackmate_ack: No regular expression found.

This was the full command (adapted for the console) which also returned the same error:

~/dev/macosapps/ackmate-git/build/Debug/AckMate.tmplugin/Contents/Resources/ackmate_ack \
  --ackmate --follow --nosmart-case --ackmate-dir-filter \
  '.*/(\.[^/]*|git|log|CVS|_darcs|_MTN|\{arch\}|blib|.*~\.nib|.*\.(framework|app|pbproj|pbxproj|xcode(proj)?|bundle||cache))$' \
  --match Proxy

I couldn't figure out any good way to get the debugger to show string values of the arguments so I opened:

"Run", "Variables View", "Show Variable as..."

for each arg and changed NSCFString to NSString ...

stepheneb commented 14 years ago

FYI: I didn't think there was an issue in the command invocation -- running ackmate project find works -- the problem is just with opening files in directories that are referenced as symbolic links -- but it was an easy way to learn how to use the debugger.

protocool commented 14 years ago

Hey,

very thorough, makes diagnosing easier :-)

I wish I had more time to delve into this but (at least until Wednesday) I'm crushed under the load of deadlines and travel.

However, what I can say is that if you get "no regular expression found" it's because you didn't specify the search directory as a the last argument on the command line

as in:

perl path_to_ackmate_ack --ackmate --follow --nosmart-case --match Proxy

will fail but

perl path_to_ackmate_ack --ackmate --follow --nosmart-case --match Proxy .

will work.

I'll revisit the issue next week when I have some breathing room.

Thanks, Trev

stepheneb commented 14 years ago

Interesting ...

When this executes in JPAckWindowController:330:

[[[NSApplication sharedApplication] delegate] openFiles:[NSArray arrayWithObject:absolute]];

and absolute references the file through a symbolic reference:

(gdb) po absolute
/Users/stephen/dev/ruby/src/em/em-http-request-git/lib/em-http/client.rb

openFileName defined just a few lines later (line 340):

openFileName = [[[wc textView] document] filename];

returns an absolute path without the symbolic link:

(gdb) po openFileName
/Users/stephen/dev/ruby/src/gems/em-http-request-git/lib/em-http/client.rb

And if I cmd-click on the filename in TM indeed the full path does not use the symbolic link.

If instead I open the file from the command line TM doesn't remove the symbolic link part of the path:

[em]$ mate .
[em]$ mate /Users/stephen/dev/ruby/src/em/em-http-request-git/lib/em-http/client.rb

The file has the correct path and is open in the project.

If I quit TM and just open the file itself:

[em]$ mate /Users/stephen/dev/ruby/src/em/em-http-request-git/lib/em-http/client.rb

it also has the correct path.

It seems the problem (conversion from path with symbolic link to one without) here is in:

[[[NSApplication sharedApplication] delegate] openFiles:[NSArray arrayWithObject:absolute]];
stepheneb commented 14 years ago

I've put a patch that fixes this (partially in a somewhat hacky way) here: http://gist.github.com/554581

I'm not going to fork and push a topic branch until I can get it to work better.

I use NSTask to run mate from the shell to open the file. This is an unfinished hack because it doesn't wait for mate to open before trying to move to the correct line.

It also doesn't check to make sure the mate shell executable is installed.

protocool commented 14 years ago

Ugh... that's inventive for sure, but really... ugh!

There has to be a better way. When I get some time I'll ask on the textmate mailing list and perhaps Alan O respond with a suggested fix.

stepheneb commented 14 years ago

I fixed the move to line number issue by using the mate '-l' argument.

Here's a topic branch with the fix: http://github.com/stepheneb/AckMate/tree/fix-opening-files-with-symbolic-links

The new code still doesn't check to see if the command-line mate executable is installed.

stepheneb commented 14 years ago

I agree it's somewhat ugly ;-)

But now this use case works for me -- and searching large numbers of files is where I really appreciate using ackmate!

I couldn't find much of any info on how openFiles using the NSApplicationDelegate protocol works with symbolic links -- this is about all I could find:

http://developer.apple.com/mac/library/documentation/cocoa/reference/NSApplicationDelegate_Protocol/Reference/Reference.html#//apple_ref/doc/uid/TP40008592-CH1-SW37

Perhaps using openFile in NSWorkspace will work differently ... probably not though.

[[NSWorkspace sharedWorkspace] openFile:absolute withApplication:@"TextMate"];

If you find a better way that would be great.

Anyway it was interesting learning a little-bit more about obj-c, Cocoa, and xcode.

Thanks for writing and releasing AckMate.

protocool commented 14 years ago

Hey - looking through the code - I realize that JPAckWindowController has a reference to projectController - that's the windowcontroller that caused AckMate's window to be opened in the first place.

Also - I see that it has an openFiles method too :-P

What if you change (in JPAckWindowController)

  [[[NSApplication sharedApplication] delegate] openFiles:[NSArray arrayWithObject:absolute]];

to

  [projectController openFiles:[NSArray arrayWithObject:absolute]];

(I'd try it myself but I'm elbows-deep in a refactoring)

Trev

stepheneb commented 14 years ago

seems to execute without error ... but doesn't open the file

protocool commented 14 years ago

Understood - will look at the issue again soon.

Thanks for digging into this.

Trev

stepheneb commented 13 years ago

No changes in the code but I have generated a pull request: https://github.com/protocool/AckMate/pull/24

I asked for advice a while ago on the textmate list but didn't get any suggestions about how to make it cleaner.

http://lists.macromates.com/textmate/2010-September/031412.html

There are two commits in this pull request. The first makes it easier to debug Ackmate while doing development. The second fixes the problem with symbolic links.