Skip to content

Conversation

@imaustink
Copy link
Contributor

@imaustink imaustink commented May 2, 2017

Fix bug breaking some Edit on Github links.
fixes canjs/can-define#176
fixes #204

@imaustink imaustink requested a review from chasenlehara May 2, 2017 20:33
if (!current.src) {
return false;
}
var name = packageObject.name,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leoj3n do you know what packageObject.name is? Is it intended to be the parent package name? So in this case canjs. Or is it meant to be the name of the current package being processed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought you were asking what is packageObject. For packageObject.name, that should simply be the name property, taken straight from package.json. So, for CanJS, it should be just "can".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afaik packageObject.name is actually the name given in the @page tag on the closest parent documentation page with a @package tag

Copy link
Member

@chasenlehara chasenlehara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@justinbmeyer
Copy link
Contributor

This seems to be already working here: https://canjs.com/doc/can-define.html Am I missing something?

var name = packageObject.name,
version = 'v' + packageObject.package.version,
srcPath = current.src.path.replace('node_modules/' + name + '/', ''),
srcPath = current.src.path.replace(SRC_PREFIX_REGEX, ''),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe why this change was necessary? What did current.src.path look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string looks like node_modules/[module_name]/*. The problem is packageObject.name is always canjs.

@imaustink
Copy link
Contributor Author

@justinbmeyer try this one for a live example of a 404

@imaustink
Copy link
Contributor Author

Closing in favor of #257

@imaustink imaustink closed this May 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Edit on GitHub leads to 404 The “source” link on the can-define/list/list doc pages doesn’t work

6 participants