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

add rendering and loading from templates/ directory, not just copying from cwd? #1

Open
tunnckoCore opened this issue Sep 13, 2016 · 6 comments

Comments

@tunnckoCore
Copy link

tunnckoCore commented Sep 13, 2016

I'm opening this one for me, cuz I'm going to PR this tonight.

Because I have this CONTRIBUTING.md file template and need rendering.

Actually, is update trying to load templates from global ~/templates directory?

edit: btw, got the idea for something like is-template-file package - sounds reasonable to have such thing? So we will be able to skip render cycle if it's not needed. Maybe streaming plugin.

@jonschlinkert
Copy link
Member

is-template-file package - sounds reasonable to have such thing?

I can't remember if ~/templates was implemented here as thoroughly as it was in generate. This is the code generate uses for resolving templates in user home:

app.onLoad(/(^|[\\\/])templates[\\\/]/, function(file, next) {
  var userDefined = app.home('templates', file.relative);
  if (utils.exists(userDefined)) {
    file.contents = fs.readFileSync(userDefined);
  } else {
    userDefined = app.home('templates', file.basename)
    if (utils.exists(userDefined)) {
      file.contents = fs.readFileSync(userDefined);
    }
  }
  if (/^templates[\\\/]/.test(file.relative)) {
    file.path = path.join(app.cwd, file.basename);
  }
  next(null, file);
});

So we will be able to skip render cycle if it's not needed

Can you clarify? Wouldn't render already be skipped if the file doesn't exist? lol

@tunnckoCore
Copy link
Author

Wouldn't render already be skipped if the file doesn't exist? lol

Not sure enough. Should review assemble-render-file again lol, but yea, maybe.

@tunnckoCore
Copy link
Author

tunnckoCore commented Sep 13, 2016

@jonschlinkert actually index.js#L55-L57 yea it will skip the file. The idea is to skip the stuff before it, options mergin and etc. So if it is not a template file we can just use app.copy instead of app.src and app.render and etc. We will save some time.

it worth, imho.

@jonschlinkert
Copy link
Member

So if it is not a template file we can just use app.copy instead of app.src and app.render and etc. We will save some time.

How would we differentiate between files that have templates variables that need to be resolved, and files that don't? might be nice to have a convention for this. I think

You can already disable rendering on individual files, I don't think options merging for a few files have any noticeable performance impact. I'm open to suggestions though

@tunnckoCore
Copy link
Author

tunnckoCore commented Sep 13, 2016

actually, i think i have idea. what about when src reads the contents check for currently used template delims and if exists, set file.data.render to true, otherwise false - then, in the renderfile we should add one more check to the file.isNull if - and it will skip da file. so it will automagically save some times.

sorry for not have formatting, but im on phone now

@tunnckoCore
Copy link
Author

tunnckoCore commented Sep 13, 2016

and to not forget, it should also be overidden if opts.render is given

edit: even if it not make sense to render if file isnt template
edit2: nevermind. this issue is for totally other thing

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

No branches or pull requests

2 participants