Set atime, mtime and ctime from contents accessors #75
Conversation
erikkemperman
force-pushed
the
feature-stats
branch
from
7c5035a to
c9f6cfc
Compare
| @@ -149,12 +149,24 @@ File.isVinyl = function(file) { | |||
| // Or stuff with extra logic | |||
| Object.defineProperty(File.prototype, 'contents', { | |||
| get: function() { | |||
| if (this.stat && this.stat.atime) { | |||
| this.stat.atime.setTime(Date.now()); | |||
There was a problem hiding this comment.
why setTime instead of an assignment? We aren't sure if the value is actually a date object, since a vinyl object could be constructed with an int for timestamp (and it should work fine, I think)
|
Added some code to the constructor which transforms integer into dates. And accompanying test. Better to use If we're worried that, post |
|
Yeah, I think that is going to be the final result, but I am good with something in the interim. Thanks. |
Set atime, mtime and ctime from contents accessors
|
@erikkemperman I was thinking this could be added as a feature, but it might be considered breaking for anyone that was relying on timestamps not changing. Do you think this warrants a major bump? |
|
@phated Actually I was about to question if the set of stats-related PRs, as it stands, doesn't need a little more thought. Some things now strike me as more a consequence of the particular order in which issues were raised than systematic -- and documented -- analysis of what should actually happen in various cases. |
|
@phated I believe the But the interaction with |
|
@erikkemperman I will avoid versioning it until we have everything nailed down in how it is supposed to work. I am running stuff through some test repos along with the unit tests. |
|
@erikkemperman I rebased this stuff out. Do you want to open it again for a lengthier discussion? |
As discussed in #1461, I think this covers the simple case.
Note: I had to adjust tests in
vinyl-fsto get them to pass againstvinylwith this patch, will submit a PR for that as well.Edit: and here it is.