-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
include mixed with filter #2929
Comments
According to this, the approach should work. I've tested this feature and it works as expected. Could you provide more info on the error? |
Thank you for the fast response. I have included below the output from
an attempt to use "include:css-clean:sass file" I have gotten similar
results from "include:uglify-js:livescript file" In both cases include
worked with either filter one at a time and both filters worked in
combination when include wasn't used. Also, a "semi-related" issue, I
attempted to your jstransformer-browserify as a workaround. I figured
that I could duplicate the feature with transforms and plugins from the
browserify ecosystem (and of course get the benefit of using "requires"
in my code) and I consistently get "browserify does not support
rendering synchronously"
result from pug myfile.pug:
/usr/local/lib/node_modules/pug-cli/node_modules/pug-filters/lib/handle-filters.js:49
throw ex;
^
Error: No input specified: provide a file name or a source string to process
at Object.module.exports.renderSync (/home/coachshea/documents/try-c/node-mutt/node
_modules/node-sass/lib/index.js:429:11)
at Object.exports.render (/home/coachshea/documents/try-c/node-mutt/node_modules/js
transformer-scss/index.js:12:18)
at Object.exports.render (/home/coachshea/documents/try-c/node-mutt/node_modules/js
transformer-sass/index.js:24:15)
at Transformer.render (/usr/local/lib/node_modules/pug-cli/node_modules/jstransform
er/index.js:286:34)
at filter (/usr/local/lib/node_modules/pug-cli/node_modules/pug-filters/lib/run-fil
ter.js:22:30)
at filterWithFallback (/usr/local/lib/node_modules/pug-cli/node_modules/pug-filters
/lib/handle-filters.js:43:18)
at /usr/local/lib/node_modules/pug-cli/node_modules/pug-filters/lib/handle-filters.
js:31:20
at Array.forEach (<anonymous>)
at walk.includeDependencies (/usr/local/lib/node_modules/pug-cli/node_modules/pug-f
ilters/lib/handle-filters.js:28:20)
at walkAST (/usr/local/lib/node_modules/pug-cli/node_modules/pug-walk/index.js:23:1
8)
Thank you,
…--
John Shea
cell: 617-605-5763
email: [email protected]
On Tue, Dec 19, 2017 at 07:52:30AM -0800, droooney wrote:
According to this, the approach should work. I've tested this feature and it
works as expected. Could you provide more info on the error?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.*
|
Yes, you are right. Include filters are applied not in reverse order but rather in the order they were written... Will do a PR today or tomorrow but don't know when it's going to be merged. Thanks for such a find that should have been detected long before :) |
By the way, you don't really need |
Thank you for your fast and helpful comments. In the meantime, it is not
a big deal to me to use them in order (although for consistency your PR
will certainly be a good thing). Additionally, (not sure if this should
be a separate issue), any idea why the browserify transform doesn't seem
to work?
Thank you again.
…--
John Shea
cell: 617-605-5763
email: [email protected]
On Tue, Dec 19, 2017 at 09:37:36AM -0800, droooney wrote:
Yes, you are right. Include filters are applied not in reverse order but rather
in the order they were written... Will do a PR today or tomorrow but don't know
when it's going to be merged. Thanks for such a find that should have been
detected long before :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.*
|
Try not to change the filters order as it's clearly a nasty bug, so either wait for the new published version or better don't use the // instead
style
include:clean-css:sass style.sass
script
include:uglify-js:livescript script.js
// write
style
include:sass(minify) style.sass
script
include:livescript(minify) script.js it even looks cleaner this way, I think. About browserify: better open another issue as it doesn't seem to be related to this one and I didn't quite get the issue, so please try to describe it a bit better there :) |
That worked perfectly. Thank you for the great advice. |
Pug Version: your version number here
2.0.0-rc.4
Node Version: your version number here
9.2.1
Input JavaScript Values
Input Pug
Expected HTML
Actual HTML
Additional Comments
I have been using filters with pug and occasionally applying multiple filters with great results. I decide to utilize the include feature with my filters and noticed that within the context of include, I can no longer include multiple filters. I have tried this with several different filters and in each case, it works without include, but not with include. I don't know if this is a bug or a feature request because I am not sure of the intended response, but it made sense to me that these should be able to work together.
Thank you for your consideration.
The text was updated successfully, but these errors were encountered: