Intended fix for Issue #216 - Changed url creation to simply join strings

[iexus]

As path.join produces backslashes on windows need an alternative to not have broken links when built on my machine.

I cannot run your tests for some reason so cannot guarantee this hasn't broken everything. I would also want to check there is nowhere else that this issue is also occurring. Not sure if the https is correct!

I thought would submit a pull request to highlight where the issue is occurring for me and to allow you to take a look.

[juandopazo]

This LGTM as a fix for #216, but there are a couple more instances were path.join() is used to create URLs and we may need to normalize base if it's loaded from external data (see https://github.com/iexus/yuidoc/blob/master/lib/builder.js#L199).

[iexus]

Agreed! I had noticed a few other points, for example when linking a property (at: https://github.com/iexus/yuidoc/blob/master/lib/builder.js#L233), not overly familiar with the code but needed something for a project yesterday so selfishly made my own changes to get something going.

[caridy]

Maybe we should simply copy the implementation we use in mojito, and use it within yuidoc everywhere we produce a url:
https://github.com/yahoo/mojito/blob/develop/lib/util.js#L46-L71

@iexus can you adjust the PR and cover the other places where join is misused?

[iexus]

Certainly, this will be a big learning experience for me all round so yes, I'll give it a go.

[caridy]

Alright, we are here to help you @iexus, don't be shy.

[iexus]

Apologies, I'm all new to this really and I was stuck till the evening at an event!

Should have said I wouldn't be able to do anything till I was back home.

Probably not quite the place for this but the thing that worries me is I still cannot run the tests, I hate to commit something I've not tested.. for example I've just noticed something I did wrong! (It's been a long day .. my mistake for committing).

Calling to npm test starts to run the script for ytestrunner but on this machine I just get all sorts of horrible errors.

[caridy]

👍

Pretty good @iexus, thanks for taking the time. I will merge this when I get a chance. If you feel that you can write a test for it, I will encourage you to do it, if not, I can do it when I merge this PR.

[juandopazo]

@iexus the npm test script just really doesn't like Windows. It'll take time to get all the YUI tools working flawlessly across OSs.

[iexus]

Thanks! Not that hard really, just need to take my time and not panic 😄

Ok, after some experimentation I appear to be getting tests to run using npm test (hooray!).

I had to remove instabul from the test script (not sure what's causing that to fail but it can't seem to look at the files). Also there seems to be an empty argument '--' straight after the ytestrunner being fed in? Is there a reason for that? Seems to try and load the other files in the /input folder where which then on my machine it declares that 'Y' is not defined.

So my test script looks like this now:

"test": "ytestrunner --include ./tests/options.js --include ./tests/builder.js --include ./tests/parser.js --include ./tests/parser_coffee.js --include ./tests/files.js"

I had to fiddle a couple of changes in some of the tests, for example in:

https://github.com/iexus/yuidoc/blob/master/tests/files.js#L28
When my tests have failed it doesn't seem to clean up the files properly - or another test hadn't removed this folder so I was getting an error here simply from trying to create an already existing directory.

And:
https://github.com/iexus/yuidoc/blob/master/tests/files.js#L149
On my system (which is not particularly old or slow mind..) it complains the directory is not empty when trying to remove it. Not quite sure but I would have though the synchronous remove would avoid that, if I put a brief wait in (of 100ms) it's fine and passes - odd.

I shall have a go at writing a brief test for the utils method I added and see if I can run it, I'll then commit just that (I'll avoid putting in what hacks I've made for getting them to run).

[juandopazo]

-- is meant to split the args so that everything after -- is passed as arguments to ytestrunner and not to istanbul.

[iexus]

Ah ok, in that case if I get istanbul working for myself then hopefully that might make things better.

I also just realised that utils.js isn't included in the tests, seems to be missing from the --include options.

[iexus]

Argh. Failure! Passed on mine, but I've just realised why that won't work on a none windows machine as a test.

[caridy]

awesome, I will merge it when I get a chance.

[caridy]

• yuidocjs@0.3.49