Skip to content
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

Jade -> HTML #3

Merged
merged 3 commits into from
Feb 27, 2017
Merged

Jade -> HTML #3

merged 3 commits into from
Feb 27, 2017

Conversation

pmpinto
Copy link
Contributor

@pmpinto pmpinto commented Feb 22, 2017

Here's the Jade task.
Please make sure it doesn't break anything. It seems to work nicely, but haven't checked the other tasks.

Also updated the README file 😊

By using 'gulpist jade' it compiles every Jade file into HTML.
Alternatively, you can use 'gulpist watch' to watch for changes
on Jade files. Only the most recently changed files will be
compiled when using this method.
By providing the extension for the compiled file
newer() is now able to compare the dates of both
files and compile it.
Copy link
Contributor

@stkao05 stkao05 left a comment

Choose a reason for hiding this comment

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

Hi @pmpinto just last couple small feedbacks. everything else is good 👍


function compileJade(onlyNewFiles) {
var options = {
pretty: config.hasOwnProperty("indentation") ? config.indentation : false
Copy link
Contributor

Choose a reason for hiding this comment

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

pretty: config.indentation ? config.indentation : false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That will do it :)

.pipe(jade(options));

if (onlyNewFiles) {
stream = stream.pipe(newer({ dest: config.dest }));
Copy link
Contributor

Choose a reason for hiding this comment

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

put newer before jade pipe

gulp.task('jade', function() { return compileJade(); });

gulp.task('jade:watch', function() {
var filesToWatch = config.hasOwnProperty("watch") ? config.watch : config.src;
Copy link
Contributor

Choose a reason for hiding this comment

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

filesToWatch = config.watch ? config.watch : config.src;


gulp.task('jade:watch', function() {
var filesToWatch = config.hasOwnProperty("watch") ? config.watch : config.src;
gulp.watch(filesToWatch, {'dot': true}, function() { compileJade(true); });
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the dot option here for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't tell, this jade:watch block was duplicated from another task you have previously created. Just wanted to make sure it worked and eventually forgot to check that.

Either way, please don't merge this just yet. I managed to create some kind of infinite loop or memory leak of some sort that completely froze my computer yesterday. I'll go over this once again and see what I missed.

@pmpinto
Copy link
Contributor Author

pmpinto commented Feb 24, 2017

For some reason I managed to switch the gulp.dest() line with the jade() line while implementing the watch feature... 😞

Should be fixed now 👍

Apparently the dot option you mentioned is used for files that start with a dot (ie: .es6.main.js). Removed it since we won't need that option for this task.

@stkao05 stkao05 merged commit dcdae67 into master Feb 27, 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.

2 participants