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

feat(broccoli): add jade render #626

Closed
wants to merge 8 commits into from
Closed

feat(broccoli): add jade render #626

wants to merge 8 commits into from

Conversation

t-fox
Copy link

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

No description provided.

@googlebot
Copy link

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
Copy link
Author

t-fox commented May 4, 2016

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@NoNameProvided
Copy link

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
Copy link
Contributor

hansl commented May 4, 2016

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 :)


options = options || {};
Plugin.call(this, inputNodes, {
cacheInclude: [/\.jade/]
Copy link
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
Copy link
Author

t-fox commented May 11, 2016

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

@filipesilva
Copy link
Contributor

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

@@ -220,6 +220,9 @@ To use one just install for example `npm install node-sass` and rename `.css` fi

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
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
Copy link
Author

t-fox commented Jun 3, 2016

@filipesilva I updated readme and rebased

@krasevych
Copy link

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

@t-fox
Copy link
Author

t-fox commented Jun 7, 2016

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

@ivanmayes
Copy link

+1 for this

@@ -367,6 +367,39 @@ describe('Basic end-to-end Workflow', function () {
});
});

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

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
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

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

@ganySA
Copy link

ganySA commented Jul 4, 2016

Has this been included?

@ShadowManu
Copy link

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

@filipesilva can this issue be revived?

@filipesilva
Copy link
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
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

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.