Suggestion on additional parameter to the `preCompile` and `postCompile` functions. #24

Closed
dnutels opened this Issue Sep 19, 2013 · 3 comments

Comments

Projects
None yet
2 participants
@dnutels

dnutels commented Sep 19, 2013

Hi.

It seems to me that there is a use case where the file (as in this.files element) that is being processed right now should be made available to the preCompile and postCompile functions.

The use case is as follows:

  • before processing the files, the metadata for all files is gathered (file names, folder structure) - in order to create some sort of documentation site, with relative paths and so on.
  • in order to create each individual HTML file from the template, it's context should have things like path or size (for example)

Unfortunately, there is no way of knowing, currently, which file is being processed.

I suggest a very simple change. Instead of passing src to the lib/markdown.js module, the entire file should be passed and then populated onto context.

templateContext.file = file;

Thoughts? :)

Also, latest changes are awesome.

@treasonx

This comment has been minimized.

Show comment Hide comment
@treasonx

treasonx Sep 19, 2013

Owner

I wouldnt want to remove src, but I think adding file to context is a legitimate request.

Owner

treasonx commented Sep 19, 2013

I wouldnt want to remove src, but I think adding file to context is a legitimate request.

@dnutels

This comment has been minimized.

Show comment Hide comment
@dnutels

dnutels Oct 2, 2013

Beacause of backward compatibility, surely?

dnutels commented Oct 2, 2013

Beacause of backward compatibility, surely?

@treasonx

This comment has been minimized.

Show comment Hide comment
@treasonx

treasonx Oct 2, 2013

Owner

I agree it is duplication, but i would hate to break compatibility for
someone depending on src, I will add a warning in the log message that it
will be removed int he future.

sent from glass

On Wed, Oct 2, 2013 at 3:49 AM, Dmitry Nutels notifications@github.comwrote:

Why wouldn't you? Your code now looks like this:

in markdown.js:

grunt.util.async.forEachLimit(this.files, 25, function (file, next) {
convert(file.src, file.dest, next);}.bind(this), this.async());
function convert(src, dest, next){
var content = markdown.markdown(
grunt.file.read(src),
options,
template
);

grunt.file.write(dest, content);
grunt.log.writeln('File "' + dest + '" created.');
next();}

and in lib/markdown.js:

exports.markdown = function(src, options, template) {...src = options.preCompile(src, templateContext) || src;html = markdown(src);html = options.postCompile(html, templateContext) || html;
templateContext.content = html;
src = _.template(template, templateContext);return src;

so you only use src and pass it around.

the change would just use file.src in all places instead of adding
another parameter:

exports.markdown = function(src, options, template, file) {

or some such, that is basically creates duplication of passed data, since src
== file.src.


Reply to this email directly or view it on GitHubhttps://github.com/treasonx/grunt-markdown/issues/24#issuecomment-25520008
.

Owner

treasonx commented Oct 2, 2013

I agree it is duplication, but i would hate to break compatibility for
someone depending on src, I will add a warning in the log message that it
will be removed int he future.

sent from glass

On Wed, Oct 2, 2013 at 3:49 AM, Dmitry Nutels notifications@github.comwrote:

Why wouldn't you? Your code now looks like this:

in markdown.js:

grunt.util.async.forEachLimit(this.files, 25, function (file, next) {
convert(file.src, file.dest, next);}.bind(this), this.async());
function convert(src, dest, next){
var content = markdown.markdown(
grunt.file.read(src),
options,
template
);

grunt.file.write(dest, content);
grunt.log.writeln('File "' + dest + '" created.');
next();}

and in lib/markdown.js:

exports.markdown = function(src, options, template) {...src = options.preCompile(src, templateContext) || src;html = markdown(src);html = options.postCompile(html, templateContext) || html;
templateContext.content = html;
src = _.template(template, templateContext);return src;

so you only use src and pass it around.

the change would just use file.src in all places instead of adding
another parameter:

exports.markdown = function(src, options, template, file) {

or some such, that is basically creates duplication of passed data, since src
== file.src.


Reply to this email directly or view it on GitHubhttps://github.com/treasonx/grunt-markdown/issues/24#issuecomment-25520008
.

@treasonx treasonx closed this May 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment