Skip to content

feat(broccoli): add jade render - #626

Closed
t-fox wants to merge 8 commits into
angular:masterfrom
t-fox:master
Closed

feat(broccoli): add jade render#626
t-fox wants to merge 8 commits into
angular:masterfrom
t-fox:master

Conversation

@t-fox

@t-fox t-fox commented May 4, 2016

Copy link
Copy Markdown

No description provided.

@googlebot

Copy link
Copy Markdown

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!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@t-fox

t-fox commented May 4, 2016

Copy link
Copy Markdown
Author

I signed it!

@googlebot

Copy link
Copy Markdown

CLAs look good, thanks!

@NoNameProvided

Copy link
Copy Markdown

Jade was renamed to pug, the transition is in progress, but I think the word jade should not be used anymore and it should be replaced with pug everywhere.

@hansl

hansl commented May 4, 2016

Copy link
Copy Markdown
Contributor

Hi. Thanks @t-fox! Could you add tests? You can base those on the CSS ones. Also, seems like linting was failing. Please fix and rebase :)

Comment thread lib/broccoli/angular-broccoli-jade.js Outdated

options = options || {};
Plugin.call(this, inputNodes, {
cacheInclude: [/\.jade/]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be /\.jade$/ (with a $ at the end) ?

@t-fox
t-fox force-pushed the master branch 5 times, most recently from a88b606 to 82233e3 Compare May 11, 2016 11:48
@t-fox

t-fox commented May 11, 2016

Copy link
Copy Markdown
Author

OK, @hansl. Added test and rebased. ;) It also uses pug instead of jade.

@filipesilva

Copy link
Copy Markdown
Contributor

@t-fox can you add some instructions to the readme regarding how to use this functionality?

Comment thread README.md
The `Angular2App`'s options argument has `sassCompiler`, `lessCompiler`, `stylusCompiler` and `compassCompiler` options that are passed directly to their respective CSS preprocessors.

### Additional Support
- pug (pug)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does this section mean?

@t-fox

t-fox commented Jun 3, 2016

Copy link
Copy Markdown
Author

@filipesilva I updated readme and rebased

@krasevych

Copy link
Copy Markdown

What about generate code with .pug ?
Because generate code with *.html and then rename to *.pug is not good(

@t-fox

t-fox commented Jun 7, 2016

Copy link
Copy Markdown
Author

@krasevych Isn't it obvious? Nonetheless I updated Readme.

@ivanmayes

Copy link
Copy Markdown

+1 for this

});
});

it.skip('Installs pug support successfully', function() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this skipped?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Other tests like sass support are skipped too since 7c0ba39

@ShadowManu

Copy link
Copy Markdown

Besides updating, what's missing for the release of this?

@ganySA

ganySA commented Jul 4, 2016

Copy link
Copy Markdown

Has this been included?

@ShadowManu

Copy link
Copy Markdown

It's still marked as a work in progress. I feel I'm still too green to contribute, but I could help out with what's missing, if original contributors can't.

@ShadowManu

Copy link
Copy Markdown

@filipesilva can this issue be revived?

@filipesilva

Copy link
Copy Markdown
Contributor

Closed as this PR was made obsolete by #1455.

I appreciate all the hard work that @t-fox put into this, but we couldn't port it into the new webpack build system as is so we decided not to merge once we started working on webpack.

@niklas-dahl

Copy link
Copy Markdown
Contributor

@filipesilva since the webpack branch is merged now, is there a plan to add a pug/jade loader, just like there is already a sass/less/stylus loader?

It would be awesome if we could just write:

@Component({
  selector: 'app-root',
  templateUrl: './app.component.pug',
  styleUrls: ['./app.component.scss']
})

@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.