-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
upgrade autoloader #1242
upgrade autoloader #1242
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bazooka07. Wow somehow I documented something without implementing it. I'm terribly sorry for that, I probably forgot.
Thanks for working on this! Please take a look at my comments.
languages_path = path; | ||
} | ||
} else { | ||
path = script.src.replace(/[\w\-]+\.js$/, 'components/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you set languages_path
here instead?
Also, you don't need to escape \-
in that case.
var config = Prism.plugins.autoloader = { | ||
languages_path: 'components/', | ||
languages_path: path, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here too, I think you meants languages_path
.
var path = script.getAttribute('data-autoloader-path').trim(); | ||
if(path != '') { | ||
languages_path = path; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this condition isn't met, we'd end up with languages_path
as undefined
. Should we have a fall back / default for languages_path
?
Yes, you are right. I fix my PR
@mAAdhaTTah @Golmote |
You have conflicts to resolve now @bazooka07 |
LGTM. Just need to rebuild the |
I'm not an expert about Git and I don't know how to resolve this conflict. So I do the following commands :
But at last, I have a conflict with prism-autoloader.min.js. Even, if I launch gulp. Can somebody help me ? |
Don't worry it's ok. I solved the conflict locally. Thanks for contributing! |
Hi Golmote,
Two points for missing components with autoloader plugin :