Support windows type builds in the guessprojectroot function

[Numkil]

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.

[codeur4]

No, same as before

[Numkil]

@tplunket thanks for taking the time to try and fix this. I just didn't have the time to look into it. Your code looks good to me!

@codeur4 Would you be so kind in testing this PR again?

[tplunket]

My pleasure, I also happened to have a Windows machine. ;-)

[hunhejj]

@Numkil I tested it and it won't work yet for Windows if the project is in the root drive. It works only after these two modifications: 1) trailing slash should be added when returning in line 231 2) it should look for len(l:searchdir)>1 in line 225 (because later a slash is added in line 228)

[Numkil]

@hunhejj Thank you for helping out, I will see into these changes. @tplunket could you try out the new lines of code I will add? Just to make sure it keeps working on everyone elses machine.

[tplunket]

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.)

[hunhejj]

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.

[tplunket]

Try to map the version in my repo. According to what Numkil posted earlier,

Plug 'tplunket/ag.vim', { 'branch': 'fixGuessProjectRootOnWindows' }

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.

[tplunket]

So that I could debug this, I put

echo 'Trying' l:item

right above the "if filereadable" line, then

call input('Found it in ' . l:searchdir)

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.

[Numkil]

I merged @tplunket's code in my pull request. It's working fine for me on all platforms. (linux, osx, windows).

[tplunket]

@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.

[tplunket]

...and pondering it, the slash needs to be added for POSIX root too, otherwise it'll try to lcd with no argument.

[tplunket]

@hunhejj give it a shot now, from my repo if you don't want to wait for @Numkil to pull.

[hunhejj]

@tplunket i pulled your branch, but it still doesn't work because of the missing slash. Just try :lcd c: on a windows vim (returns the error "can't find direcotry "c:" in cdpath"). Adding the slash before returning really solves the problem.

[Numkil]

I merged this as well. @hunhejj @tplunket I guess it should be ready now :)

[hunhejj]

@Numkil nice, thanks!

[tplunket]

@hunhejj I added the slashes, or at least I tried to... Any feedback on what the failure path in the code is?

[tplunket]

@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.

[Numkil]

Tried it myself and it does work for me too. @hunhejj If you could verify if everything works as expected now that would be great. @losingkeys This pull request should be almost ready. Could you merge it when @hunhejj verifies it works?

[hunhejj]

@hunhejj, @tplunket, @losingkeys Absolutely, it works perfectly on Windows. That is why I thanked for your solution in my last comment.

[Numkil]

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?

[losingkeys]

@Numkil yeah I'll try to test it out on my windows box soon here. Thanks for all your hard work!

[hunhejj]

@losingkeys any progress?

[Numkil]

@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...

[tplunket]

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.

[hunhejj]

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.

[losingkeys]

@hunhejj, @Numkil, @tplunket: sorry if it wasn't clear. I didn't want to close the issues in case people wanted to migrate them. Let me know if the README could have more clarification as to why this has been deprecated.