-
Notifications
You must be signed in to change notification settings - Fork 906
small changes so that it can be used directly in a npm pipeline #476
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
Just some updates. I've successfully used my fork as a npm package ( You can check:
Examples of usage:
Can someone check if there are potential side effects from the changes I made so that we can merge the PR? If there are we can discuss them so that I can make the needed modifications. //cc @amroamroamro @mikesamuel |
btw. I created a gitter chat room for my fork at https://gitter.im/alex-milanov/code-prettify (You guys could create an "official" one or join the one I created, both options work for me) |
@alex-milanov Thanks for this ! Why isn't this merged with the original project yet ? |
@ateev beats me. //cc @amroamroamro @mikesamuel |
Change to prettify.js for better React support
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
@yuschick seems you have to sign a google CLA (Contributor License Agreement). |
src/prettify.js
Outdated
@@ -141,7 +141,8 @@ var PR; | |||
* UI events. | |||
* If set to {@code false}, {@code prettyPrint()} is synchronous. | |||
*/ | |||
window['PR_SHOULD_USE_CONTINUATION'] = true; | |||
var PR_SHOULD_USE_CONTINUATION = true; | |||
if (typeof window !== 'undefined') window.PR_SHOULD_USE_CONTINUATION = PR_SHOULD_USE_CONTINUATION; |
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.
I think these changes need to be made in https://github.com/google/code-prettify/blob/master/js-modules/prettify.js#L94
Please also use brackets around if bodies.
Also, please use ['...'] form so that it is closure-compiler compatible.
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.
Sure. I will check it out. btw. wouldn't it be more logical to have the sources in the src
folder and the destination in a dist
one?
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.
@mikesamuel also aren't these vars constants -> can we use
const PR_SHOULD_USE_CONTINUATION = true;
src/prettify.js
Outdated
@@ -1740,3 +1741,14 @@ var prettyPrint; | |||
}); | |||
} | |||
})(); | |||
|
|||
// npm require support | |||
if (module && module.exports) module.exports = { |
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.
Won't if(module) throw an exception if there is no global named "module".
Maybe if (typeof module !== 'undefined' && module.exports)
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.
Probably it shouldn't be an issue in most cases but the second one would be more consistent.
@mikesamuel after struggling for a while with the closure compile process and stumbling upon how run_prettify is formed found a way to add node support by introducing |
@mikesamuel I hope we won't have to wait 6 more months for a response :-) |
Bump. |
Looks good to me. Let's merge this in. |
Thanks for doing this. Sorry for taking so long to get back to it. |
Thank you everyone!!! 🙌🎉 |
…ushed to master 3 Nov. This is so that it can be used as a npm module. googlearchive/code-prettify#476
Any possibility of doing a new public release to help get this feature out into the wild? 🦁 |
If you need help I am available |
usage in a node module
usage in a task automation (like gulp, grunt...) for css preprocessor (like sass, less)
then inside a sass file