Skip to content
This repository was archived by the owner on Apr 22, 2020. It is now read-only.

Conversation

amroamroamro
Copy link
Contributor

@amroamroamro amroamroamro commented Jan 18, 2017

Summary:

  • created package.json and bower.json for integrating with npm and Bower package managers. Note: I set the version to "1.0.0", feel free to change it to something else if needed.
  • replaced functionality of the makefile with grunt. I added grunt tasks to match make targets, including:
    • grunt preprocess to process js-modules/*.js files and produce src/prettify.js and src/run_prettify.js bundles
    • grunt gcc to compile/minify JS using Closure Compiler (also added a grunt uglify task as alternative, but it's not used by default)
    • grunt cssmin to minify CSS files
    • grunt aliases to create lang aliases
    • grunt compress to produce a ZIP file for distribution
    • grunt clean to clean generated files

To rebuild the project from scratch, first you need to install Node, then you simply run :

$ cd code-prettify
$ npm install
$ npm run grunt

(note: Closure Compiler requires Java JRE to run).

ran 'npm init' and 'bower init' and filled some fields
we're a using package which processes @include directives
(similar to server-side-includes)
We also use @ifdef/@ifndef to customize prettify and run_prettify
replace PR.PR_PLAIN with "pln", etc.
This is needed because closure-compiler:langs will attempt to compile
all 30+ lang files in parallel, each being a separate target.
The concurrent Java processes will struggle for memory and crash.

This task will split the multi-file target into separate targets, each
with one file. This way the lang files are compiled one by one.
functionality replaced with grunt
this way we dont need to have grunt-cli globally installed,
we can just use: "npm install && npm run grunt"
(Note: npm sets the PATH correctly to find ./node_modules/.bin/grunt)
var IN_GLOBAL_SCOPE = true;
/* @endif */
/* @ifdef RUN_PRETTIFY */
var IN_GLOBAL_SCOPE = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this cause duplicate definition warnings under closure compiler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it won't because the file is never fed directly to closure compiler, it only sees src/prettify.js and src/run_prettify.js by which time those directives are already "processed" and only one of the two alternatives survives (either true or false depending on wheather RUN_PRETTIFY was set in the context (see the grunt preprocess task options)).

In fact the value used here doesn't really matter (I set it here only as a form of documentation), because we later pass the closure-compiler option --define='IN_GLOBAL_SCOPE=??' which overrides the const anyway: https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler#define-type-description

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood. LGTM

print "$pattern;\n";

print "
// CAVEAT: this does not properly handle the case where a regular
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self. Caveat moved to top file comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that way it shows up above the regex it's talking about when included in the final bundle.

include("defs.js");

var HACK_TO_FIX_JS_INCLUDE_PL;
/* @ifndef RUN_PRETTIFY */
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: run_prettify includes defs.js above its envelope.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I understand this is a limitation of closure compiler: https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler#typedef-type, so I tried to keep things the same as what js_include.pl was doing, so that defs.js typedefs are always included top-level in both prettify.js and run_prettify.js

"Mike Samuel <[email protected]>"
],
"contributors": [
"Amro <[email protected]>"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more accurate to put you down as a maintainer these days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

* separate targets (one file per target) so that lang files are compiled
* sequentially (sync) instead of in parallel (async).
*/
grunt.registerMultiTask('gcc', 'Override closure-compiler', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

It blows my mind that someone would use "gcc" as an abbreviation for Google Closure Compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hehe, what other GNU compiler thing :)

@mikesamuel mikesamuel merged commit f6c4ce7 into googlearchive:master Jan 20, 2017
@amroamroamro amroamroamro deleted the patch-grunt branch January 20, 2017 14:08
@graingert
Copy link
Contributor

@mikesamuel you forgot to publish this to npm. Now the package name has been stolen.

@amroamroamro
Copy link
Contributor Author

I didn't realize this was the wild west!

@graingert
Copy link
Contributor

@mikesamuel @amroamroamro is this going to be fixed?

@amroamroamro
Copy link
Contributor Author

@graingert I noticed you published the package under the code-prettify name. Was that to protect the name before others grab it?

@graingert
Copy link
Contributor

@amroamroamro it was to steal it! But yeah I'll hand it over if you promise to be more careful next time, and one of the members gives me their preferred npm ID.

@tamj0rd2
Copy link

tamj0rd2 commented Feb 14, 2017

Can we get some details on how to use code-prettify with npm?

@graingert
Copy link
Contributor

@amroamroamro you've not been forthcoming with any npm IDs...

@graingert
Copy link
Contributor

@tamj0rd2 I would not recommend using the npm edition of this tool until the official team have some ownership over the module name.

@amroamroamro
Copy link
Contributor Author

@graingert I ran npm adduser and created amroamroamro.

@amroamroamro
Copy link
Contributor Author

amroamroamro commented Feb 15, 2017

@tamj0rd2 If I'm not mistaken, you can always install directly from GitHub repo:

npm install google/code-prettify

or specify the full URL: https://github.com/google/code-prettify

@graingert
Copy link
Contributor

Yeah but that breaks npm shrinkwrap

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants