-
Notifications
You must be signed in to change notification settings - Fork 131
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
Support windows type builds in the guessprojectroot function #112
Conversation
No, same as before |
Support windows type builds in the guessprojectroot function
…lashes in non-Windows paths
Fix for guessProjectRoot() on Windows
My pleasure, I also happened to have a Windows machine. ;-) |
@Numkil I tested it and it won't work yet for Windows if the project is in the root drive.
|
…e logic. Fixes formatting.
Returning a trailing slash from s:guessProjectRoot is unnecessary. The only thing it's used for is the argument to 'lcd'. There's a comment documenting why len(l:searchdir) is checked to be greater than 2; that was basically the previous behavior but on Windows it doesn't allow you to have an entire drive under version control. Any folder at root should still be searched, e.g. C:\A is a directory name that would pass the test. I don't see where the later slash is added; in the PR line 228 is a comment. I've just reworked that logic though to unify it between Windows and POSIX systems. There's a PR waiting for Numkil to evaluate. :) (Didn't see Numkil's message before starting.) |
You can have an entire drive under version control, if you map a folder to a drive (this is my case). I didn't dig deep into the code, however with the above 2 changes it works, and without any(!) of them, it won't, therefore the trailing slash is somehow important. |
Try to map the version in my repo. According to what Numkil posted earlier,
I don't use any plugin managers either but handles going all of the way to the root of the filesystem on both Windows and POSIX. |
So that I could debug this, I put
right above the "if filereadable" line, then
right before "return l:searchdir" and another 'call input' right before the failure case return at the end of the function. If it's still not working maybe these will help us figure out why. I actually have some code to allow you to provide your own function for the searching (in my master branch) and have pondered allowing you to specify the list of things to look for (e.g. I always have a file named 'tags' in the project root) but haven't been motivated to do the documentation to finish them up. |
Tighten up guessProjectRoot for Windows
I merged @tplunket's code in my pull request. It's working fine for me on all platforms. (linux, osx, windows). |
@hunhejj due to the way Windows directory changing works, yes, the trailing slash is required at the root... Ugh, ok I'll get another patch in for that. I hadn't thought of testing it with a subst'd drive. |
...and pondering it, the slash needs to be added for POSIX root too, otherwise it'll try to |
…nfo. Fixes changing to root directory if the entire volume is under source control.
@tplunket i pulled your branch, but it still doesn't work because of the missing slash. |
Returns a trailing slash on the directory name holding project root
@Numkil nice, thanks! |
@hunhejj I added the slashes, or at least I tried to... Any feedback on what the failure path in the code is? |
@hunhejj I did it on Mac and just now tried it on Windows, subst'ing a drive, and it switched directories to the root just fine. |
Tried it myself and it does work for me too. |
@hunhejj, @tplunket, @losingkeys Absolutely, it works perfectly on Windows. That is why I thanked for your solution in my last comment. |
Hey @losingkeys this pr has been finished for a while now and it's necessary to remove an existing bug from the project. Could you look into this? |
@Numkil yeah I'll try to test it out on my windows box soon here. Thanks for all your hard work! |
@losingkeys any progress? |
@hunhejj. @losingkeys deprecated ag.vim completely and said he won't be working on it anymore again. so we will likely never see it merged... |
I'm not sure where @Numkil's various branches are but @hunhejj if you want to pull from my master (https://github.com/tplunket/ag.vim) branch, it's only missing the deprecation notice and some help text about a 'ga' motion that doesn't seem to have been implemented. I'll keep fixing any issues that come up over there and I'm not going to fall victim to that weird situation that they created for themselves in the original repo. |
oh, I wasn't aware that he deprecated his project. Thank you @tplunket for the hint! Actually I have a working version already, I was just courious why it was never merged. But now I understand. |
Probably fixes #111. @codeur4 could you check out this PR for me and try it out? I don't have a windows pc in my possession.