mikeal / node.couchapp.js

Utility for writing couchapps.
Apache License 2.0
406 stars 78 forks source link

A very small change: just to Add windows support :) #43

Closed osher closed 12 years ago

osher commented 12 years ago

on windows - process.env.PWD returns "undefined".

The proposed alternative is process.cwd().

So I add a fallback to that :)

mikeal commented 12 years ago

does process.cwd() work on both posix and windows? cause we could remove the || and just use that.

osher commented 12 years ago

I'm humble to say I did not try. You're right that just adding safety without chekcing is dirty, but I won't establish an environment just for that... I now have only windows, I'm on WinXP, with node 0.6.9 (installer), and GoW 0.4.0 (installer) - have no idea about cywig (yuch...). I can have a team mate check the same settings with Windows7.

Thanks for picking up. BTW - I just saw that pull request 41 says the same and does it better...

P.S - I'm not sure its related - however - it just might, maybe it should be a new issue - I'm having a trouble when couchapp is installed with -g. When its installed locally to the project - it works... The error is can't find modle "couchapp" on the line of require in the user's app.js. Maybe it's a windows NPM thing...

mikeal commented 12 years ago

i stopped doing global installs of anything, if you add ./node_modules/.bin to your PATH you can use the bin whenever it's installed.

mikeal commented 12 years ago

could you update the branch this pull is against to just use .cwd() for everything, then i'll merge.

mikeal commented 12 years ago

whoops, hit close accidentally.

osher commented 12 years ago

Done. I added both fixes to the same branch

(don't know why it started a new one from the first place... I'm new to this git-hub, but I'm learning... - could use some help to cool reference links)

UPDATE: uh... I just saw you pulled the main@183 from its other pull request. Nissee.. I hope that redoing it here won't cause problems...

osher commented 12 years ago

about:

mikel sais:

"i stopped doing global installs of anything, if you add ./node_modules/.bin to your PATH you can use the bin whenever it's installed."

I did not quite completely followed... let me see if I understand: 1 - "i stopped doing global installs of anything" = "manage your env variables by yourself" 2 - "if you add ... whenever it's installed." = install couchapp locally to every project did I get you right?

In fact, when its installed localy, it works without any path changes. But I would like to not install it locally to any project, but - I try to use it with npm install couchapp -g

If we can get it to work like that - we're soooo winners...! I'm a system architect of few teams, and have been working hard to push node and couchapp to my chief architect, who's not too patient for cutting-edges... Getting this one step forward will do us a lot of good, and not only to my projects :)

I understand that we'll first have to get it ripe, and then update the NPM package (that I only know theoretically about it, but never saw how it's done and what it actually means).

I'd love to help more :)

osher commented 12 years ago

One more issue:

When using boiler - again winXP, Gow - I get one error message on the folder of the project that I try to create.

The error occures when tried like the output bellow:

D:\projects\couchapp boiler oups
Could not create D:\projects\oups

D:\ws\OSGE\1000\sources\Analytics>

And when used like this - the same:

D:\projects>mkdir lala

D:\projects>cd lala

D:\projects\lala>couchapp boiler
Could not create D:\projects\lala

D:\projects\lala>

That led me to see that the boiler tries to create the folder, and then, the copytree() tries to create it as well. We're talking about main.js, line 37~45.

If this behavior is observed only on windows - we should not create the folder then, and let the copytree do it for us.

A second approach will be to either suppress EEXISTS errors on the copytree function, or check existing before creation. extending the console.log at bin.js@22 in function copytree(...) to

       console.log('Could not create '+dest + ":\n" + JSON.stringify(e))
Could not create D:\projects\lala: 
{"errno":47,"code":"EEXIST","path":"D:\\projects\\lala","syscall":"mkdir"}

Thoughts?

osher commented 12 years ago

Any news?