metalsmith / layouts

A metalsmith plugin for layouts
MIT License
116 stars 49 forks source link

Add matchBase to test against file Path #54

Closed dacodekid closed 8 years ago

dacodekid commented 8 years ago

Without matchBase option, multimatch wouldn't test against paths.

ismay commented 8 years ago

Thanks for the PR. The way I understand it, setting matchbase to true will mean that patterns without slashes will only be matched against the basename of the path (if the path contains slashes). See here: https://github.com/isaacs/minimatch#matchbase.

So with matchBase: true it'll assume that a pattern without slashes is meant to concern filenames only. This might break people's patterns. For example, a pattern of a.jpg will then match all a.jpg files, regardless of their nesting. So I'm not sure that this is a good idea, unless it fixes something.

What exact problem were you having that would be fixed by this?

dacodekid commented 8 years ago

@ismay, plz consider this example from multimatch - https://github.com/sindresorhus/multimatch/blob/master/test.js#L202-L210

With a setting like below, the plugin always return false and never pass my posts.jade file.

    .use(layout({
      directory: 'layout',
      default: 'post.jade',
      pattern: ['*.html', '*.jade'],
      engine: 'jade',
      pretty: true,
      moment: moment
    }))

By removing matchBase from the check.js file, all I get is just the `html from markdown.

posts/testpost.html

<p>This is just a from test post</p>

Besides, I moved from Metasmith to my own custom build tool, but would happy to help if you need further testing.

ismay commented 8 years ago

plz consider this example from multimatch - https://github.com/sindresorhus/multimatch/blob/master/test.js#L202-L210

Yes, I've read that, and understand what the difference is. However, what you're trying to fix isn't completely clear to me. In your original issue you mentioned:

This needs to be changed to if (pattern && !match(file, pattern, {matchBase: true})[0]) { in order to escape / from the file name

But the filenames from the files object don't contain a starting slash, so that shouldn't be necessary. In the example you gave above several things are still unclear, so it would be very helpful if you could point me to a repository with a minimal reproduction of the bug.

Hope that's not to much trouble, but it's difficult for me to know what you're trying to fix without seeing what bug it concerns.

dacodekid commented 8 years ago

@ismay - pushed a sample site https://github.com/dacodekid/metal-blog. May be my approach here is wrong, but plz take a look and let me know. For this one, the index.html will be generated fine, but the testpost.md that's inside the posts is not parsed through it jade template.

ismay commented 8 years ago

Thanks! I took a look, and what you're trying to do will work if you change line 32:

pattern: ['*.html', '*.jade']

to

pattern: ['**/*.html']

So the current matchBase setting requires you to be a little more explicit with your patterns. Also, *.jade isn't necessary with the files you have in src atm. and layouts doesn't process templating syntax in files in src, so I'd remove that.

I can see how changing the matchBase default would allow your old pattern to work, but it could potentially be a breaking change for other people. Furthermore, the current behaviour is not a bug but by choice so I'm closing this PR and the related issue. Nevertheless thanks for the report and feel free to reply if you have any further questions!

dacodekid commented 8 years ago

That works :dancer: But I couldn't understand what you mean by

Also, *.jade isn't necessary with the files you have in src atm. and layouts doesn't process templating syntax in files in src, so I'd remove that.

Could you please point me to that?

ismay commented 8 years ago

Metalsmith-layouts only processes templating syntax in the files in your layouts folder. So for example, if there are any variables in posts/testpost.html they won't be processed.

If you want to do that you can use metalsmith-in-place.

dacodekid commented 8 years ago

:+1: thx :)

ismay commented 8 years ago

Np :v: