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

Should it add file.origin property #19

Closed
popomore opened this issue May 27, 2014 · 15 comments
Closed

Should it add file.origin property #19

popomore opened this issue May 27, 2014 · 15 comments

Comments

@popomore
Copy link
Contributor

Some plugin like gulp-rev will change file.path, and save the original path with file.revOrigPath.

I think it's nice that supporting by vinyl.

@yocontra
Copy link
Member

This seems non-standard - what are the reasons for adding this?

@popomore
Copy link
Contributor Author

Yeah, but it can be standard.

If I use some plugin that change the path, and the plugins will save origin path by different name. I don't know where is the real path.

gulp.src('*.less') // a.less
  .pipe(less()) // change a.css, save file.lessOriginPath
  .pipe(rev()) // change a-592823.css. save file.revOriginPath
  .pipe(myPlugin()) // want to get origin file.path

@yocontra
Copy link
Member

Can you give an example of a plugin?

@popomore
Copy link
Contributor Author

popomore commented Jun 6, 2014

Sorry for late response.

For example, write a myPlugin above, I want to generate a map that refer to the original file.

module.exports = function(filename) {
  var map = {};
  throught.obj(function(file, enc, cb) {
    map[file.originPath] = file.path;
    this.push(file);
    cb();
  }, function(cb) {
    fs.writeFileSync(filename, JSON.stringify(map));
    cb();
  });
};

yield

{
  "a.less": "a-592823.css"
}

@popomore
Copy link
Contributor Author

@contra can you see it?

@dgbeck
Copy link

dgbeck commented Jun 28, 2014

The same underlying issue is described in #17.

The original path is a missing concept in the vinyl stream object. Vinyl is billed as a virtual file format - there needs to a way figure out reliably to which real file the virtual file corresponds. Clearly this information is useful and there is nowhere else to get it from.

The solution proposed here is also cool in that does not break backwards compatibility. Implementation seems straight forward. I'd like to see this problem solved. @contra will you consider a PR?

@yocontra
Copy link
Member

It's not that I don't agree, but I don't think this goes far enough. Do you think every time the path is modified we should keep a history of that? We can have a setter on the path field that pushes the previous version to a history array. To get the original one you can do history[0]

@popomore
Copy link
Contributor Author

that's a good idea—
Sent from Mailbox for iPhone

On Sat, Jun 28, 2014 at 3:23 PM, Eric Schoffstall
[email protected] wrote:

It's not that I don't agree, but I don't think this goes far enough. Do you think every time the path is modified we should keep a history of that? We can have a setter on the path field that pushes the previous version to a history array. To get the original one you can do history[0]

Reply to this email directly or view it on GitHub:
#19 (comment)

@dgbeck
Copy link

dgbeck commented Jun 28, 2014

I see. That would certainly solve the issue and is future-proof.

I dunno though... Since the intermediate names don't really point to anywhere real I don't see why they would be needed. @popomore to be clear for your use case you just need the original names right?

Thinking out load here, looking just at the semantics it seems like there are three separate concepts..

  1. The full path of the original file (read only). That is needed for maps and some other use cases where it is important to be able to trace a vinyl object back to its source file.
  2. The output file extension (read / write). This is needed so that transforms can manipulate it and then the output file has the correct extension. (It can also be used a bit awkwardly to avoid double transforms, like when the same transform is applied locally and globally.)
  3. The output file name (read / write). This is needed when the transform needs to change the name of the file for some reason, one use case being the rev() transform above.

(2) and (3) can be combined into one property, which is effectively the dual purpose the path property currently serves. (It seems like a bit of a misnomer since the directory part of the path is not used, but it gets the job done.) Adding an originalPath property would take care of (1).

I can't think of a case where the intermediate values would be useful further done the pipe. Once they have been changed they don't refer to anything useful or persistent. Would a transform every need to know that a file was of a certain type at some point but no longer? Maybe for the the double transform issue mentioned in (2) above. Seems very frail to rely on file extensions like that though, probably doesn't make sense to cater to that awkward use case.

Unless there is a good use case at hand I'd say best to keep it simple with a single property. Has the advantage of not requiring that existing transforms be updated to push to history. Either approach would solve the issue at hand though.

Thanks for looking at this @contra ! Let me know if you'd like a PR for either approach.

@sindresorhus
Copy link

I agree with @dgbeck. I only see a use-case for the originalPath.

@popomore
Copy link
Contributor Author

Yeah, but history can record every change, maybe it's useful in some complex building system.

@yocontra
Copy link
Member

I only see a use-case for the original path, but I think all of the info could be useful in visualizing builds and debugging

popomore added a commit to popomore/vinyl that referenced this issue Jun 28, 2014
@sindresorhus
Copy link

but I think all of the info could be useful in visualizing builds and debugging

I don't. We can add it later if there's an actual need, but right now it's just unnecessary bloat.

@yocontra
Copy link
Member

undecided

future proof or bloat, undecided

@popomore
Copy link
Contributor Author

Have been merged

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

No branches or pull requests

4 participants