It doesn't work on Windows

[PhiLhoSoft]

OK, title is a bit provocative, I should add "out of the box"...

First, thanks for the plugin: we use Perforce at work, and hand-checking out these pesky read-only files would be a pain... I used Brackets previously, it has a similar plugin (but more primitive), it is very useful (although intermittently failing, probably because P4 doesn't answer fast enough).

I use Atom 1.3.1 on Win7. P4 is in the path. It is located at: C:\Program Files (x86)\Perforce\p4.EXE

First, I see in the settings of your package: You probably wanted to put C:\Program Files\Perforce but the backslashes were lost... But it is supposed to pick p4 from path, alas it doesn't do it.

I pasted the above path (without p4.exe), but it still doesn't work. I changed the backslashes to forward slashes, without success. Tried with double quotes around the path, no luck.

Looked at console, found the following:

Error: spawn C:\Windows\system32\cmd.exe ENOENT(…) -- C:\Users\plhoste.atom\packages\atom-perforce\lib\atom-perforce.js:570 Error: spawn C:\Windows\system32\cmd.exe ENOENT(…) -- C:\Users\plhoste.atom\packages\atom-perforce\lib\atom-perforce.js:611 Error: spawn C:\Windows\system32\cmd.exe ENOENT(…) C:\Sources\ea\designer\service\service-designer\src\main\webapp\app\components\templateManagement is outside any known perforce workspace -- C:\Users\plhoste.atom\packages\atom-perforce\lib\atom-perforce.js:187 Error: spawn C:\Windows\system32\cmd.exe ENOENT(…)

Note: the file IS in a Perforce workspace, of course. I get the message when I manually do Packages > Perforce > Edit.

Atom shows the following message when I save with auto-edit activated:

Unable to save file 'C:\Sources\someProject\src\main\webapp\app\components\templateManagement\templateCreationController.js' EPERM: operation not permitted, open 'C:\Sources\someProject\src\main\webapp\app\components\templateManagement\templateCreationController.js'

OK, I will take a look at the source code, but I promise nothing: I am not a Node.js dev, neither an Atom one!

[PhiLhoSoft]

settings.js: defaultWindowsP4Directory = 'C:\Program Files\Perforce' These backslashes are not valid! Must be doubled, \P isn't a valid escape...

[PhiLhoSoft]

OK, found an issue, line 169 of atom-perforce.js p4Info.currentDirectory.toLowerCase().startsWith(p4Info.clientRoot.toLowerCase()) p4Info.currentDirectory is "c:\Sources\someProject\somePath\etc" p4Info.clientRoot is "C:/Sources" Note that my P4ROOT environment variable is C:\Sources...

The paths should be normalized at some point (and you should pull out this test to its own function, I see it in all commands...).

[unional]

Let me take a look into it. I introduced that code.

But why would you have C:/Source...that's not normal in Windows.

[unional]

@mattsawyer77 , do you mind if I add testing using mocha and chai in your project?

[unional]

FYI, my p4 on windows server 2012 R2 is installed at C:\Program Files\Perforce\p4.exe. So just checking platform seems to be not sufficient. Considering where p4 on windows.

[mattsawyer77]

@unional that would be awesome. thanks for all your help!

[unional]

@PhiLhoSoft , it should work when @mattsawyer77 merge #49 . I originally didn't pull it into its own function because I don't want to introduce extra changes. But now it is . :)

[PhiLhoSoft]

I made some changes to see if I can make it to work... At least, it was interesting to see I can easily hack and debug an Atom package... :smile:

I can confirm it works after normailizing the slashes. I still see an error at start on p4 info, I will investigate. (Or perhaps I got it because I wasn't logged in yet in Perforce? Will check.)

function normalizePath(path) {
    return path.toLowerCase().replace(/\\/g, '/');
}

function checkIsInWorkspace(p4Info) {
    return !p4Info['clientUnknown.'] && normalizePath(p4Info.currentDirectory).startsWith(normalizePath(p4Info.clientRoot));
}

and called checkIsInWorkspace(p4Info) everywhere it is used (there is a reversed condition, I just ! it...).

But why would you have C:/Source...that's not normal in Windows.

Well, this style of path works in Windows in 99 % of cases, so it is safer to rely on it...

Considering where p4 on windows.

Would work on my computer, as I installed Unix utilities (UnxUtils or GnuWin32), but it won't work, AFAIK, on most Windows computers.

Note: after my hack, the package works with p4 in the path (ie. I removed the path from the settings), so just keep the current behavior as is (just fix these pseudo escape keys in the string! :smile:). It is highly probable to find p4 in the path anyway.

[unional]

Oh, didn't know where p4 might not work. Will test it on another system tomorrow. Thanks! On Thu, Dec 17, 2015 at 12:51 AM Philippe Lhoste notifications@github.com wrote:

I made some changes to see if I can make it to work... At least, it was interesting to see I can easily hack and debug an Atom package... [image: :smile:]

I can confirm it works after normailizing the slashes. I still see an error at start on p4 info, I will investigate. (Or perhaps I got it because I wasn't logged in yet in Perforce? Will check.)

function normalizePath(path) { return path.toLowerCase().replace(/\/g, '/'); }

function checkIsInWorkspace(p4Info) { return !p4Info['clientUnknown.'] && normalizePath(p4Info.currentDirectory).startsWith(normalizePath(p4Info.clientRoot)); }

and called checkIsInWorkspace(p4Info) everywhere it is used (there is a reversed condition, I just ! it...).

But why would you have C:/Source...that's not normal in Windows.

Well, this style of path works in Windows in 99 % of cases, so it is safer to rely on it...

Considering where p4 on windows.

Would work on my computer, as I installed Unix utilities (UnxUtils or GnuWin32), but it won't work, AFAIK, on most Windows computers.

Note: after my hack, the package works with p4 in the path (ie. I removed the path from the settings), so just keep the current behavior as is (just fix these pseudo escape keys in the string! [image: :smile:]).

— Reply to this email directly or view it on GitHub https://github.com/mattsawyer77/atom-perforce/issues/46#issuecomment-165386442 .

[PhiLhoSoft]

Errors on startup: I get them on the getOpenedFiles call, and on the showClientName one. Both fail because they call p4 info on a path that isn't legal: atom://about. I think I get the latter because Atom display an About page on startup, and I haven't figured out yet how to get rid of it, but it should be handled anyway. Perhaps with a test if the path starts with atom: to avoid this special scheme...

Note: I also suggest to avoid doing a console.log() of results on the getChanges call: no need to spam the console with lengthly results...

[PhiLhoSoft]

OK, i no longer have errors in the console, and I can check out files. I am a bit in a hurry, and won't have time to do a real PR, so I just pasted the file with my changes at https://gist.github.com/PhiLhoSoft/091f05e78e6df3472096

Just take inspiration from these changes, perhaps, improve them (they are a bit hasty, etc.). Feel free to use at will.

[unional]

Fix is in #50 . :)

[PhiLhoSoft]

I updated the package, and I can confirm it works out of the box, now. So you can close the issue.

But I suggest to remove lines like console.log(result); to avoid spamming the console. Thanks.