Allow filters to read non-text include files#3213
Allow filters to read non-text include files#3213ForbesLindesay merged 10 commits intopugjs:masterfrom
Conversation
|
I rebased out 1ccbcfa: it was a fix to what I thought were broken tests, but I guess my local setup is different from CI. |
ForbesLindesay
left a comment
There was a problem hiding this comment.
I think a cleaner approach to this would be to do it as part of the filter. i.e. we could allow you to pass in a filter that looks like:
// options.js
exports.filters = {
png: {renderBuffer: function(data, options) { /* do something heinous */ }},
};We could check to see if a custom filter of that shape was passed when we're including files.
pug (2.0.4 → 3.0.0)Breaking Changes
New Features
pug-filters (3.1.1 → 3.2.0)New Features
pug-load (2.0.12 → 3.0.0)Breaking Changes
New Features
Packages With No ChangesThe following packages have no user facing changes, so won't be released:
|
|
Thanks for the reply! I was going for the absolute smallest change to accomplish what I needed.
Okay, sounds reasonable. In order to add a magic key like At this point, the file has already been read by I also don't know all the other cases where a filter assumes its input is plain text, this I guess what I'm asking is: could you give me a bit more guidance on how to safely add a |
This allows Pug to correctly read non-utf8 files, if the user wants to do such a thing. The optional function takes the same arguments as load.read(), and should return a truthy/falsey value. If truthy, load.read() will NOT specify an encoding when it reads the file from disk. Otherwise, it will continue to specify "utf-8" as the encoding.
|
I think the process should be:
This will be a breaking change for pug-load, but should be non-breaking for everything else. |
I mean, right now as a new option it's non-breaking to everyone. :P But yes, I do see the value in having this be a default, rather than some buried option. I'll see if I can manage it by changing |
This lives next to the existing `file.str` property.
|
OK, here's another attempt, slightly modified from your steps. At first I tried your steps as-is, but step 3 (where I probably totally botched modifying https://github.com/pugjs/pug/blob/master/packages/pug-load/index.js#L28) completely exploded the tests with 55 failures, so I got spooked and went back to trying to change as few things as possible. So I added a second property to |
|
Please educate me. |
These render as objects with `type: "Buffer"`, along with the `size` and `hash` of the data in the buffer.
|
@trasherdk Yes, but only via a specially-crafted filter. No current pug users are going to start seeing Here's a small example: // options.js
exports.filters = {
png: {
// instead of a function, specify an object with a "renderBuffer" property
// whose value is a function that takes a Buffer instead of a string
renderBuffer: function(buffer, options) {
var data = Buffer.from(buffer).toString('base64');
return '<img src="data:image/png;base64, ' + data + '"/>';
}
}
};// foo.pug
include:png my-small-image.png |
ForbesLindesay
left a comment
There was a problem hiding this comment.
This looks good to me. @brewingcode can you just confirm that the change log (see the comment from Rolling Versions) looks correct, and then I'll merge this so it goes into the next release.
|
@ForbesLindesay thanks! I made the comment scan a little more naturally in the example options.js notes for the Rolling Version and hit "Save Changes". I also edited my comment above to match. If there's anything else, lemme know! |
I have some custom filters that I use with Pug's
include, but the filters were choking because the incoming "text" from theincludewas wrong: it was automatically encoded asutf8but my filters needBuffer.So the simplest thing was to just expose a new option that, given the arguments to
load.read, would simply tell Pug to either continue specifyingutf8encoding, or just leave the encoding out and return theBuffer.A very stripped-down example looks like this:
// my-template.pug include:png image.pngI also noticed one broken test (unrelated to my changes) and fixed it, too.
This use-case is minor enough that I didn't even want to mention it in any docs.