Closed demurgos closed 8 years ago
I saw this, but it is listed as an option, not a property.
It does not state if it is then directly accessible without breaking anything. If it is editable, which values are accepted ? The same as when passed as an option ?
It's only an option, not a property. It is assigned just like any other property. It is currently documented properly. If you have improvements, submit a PR for review.
Well my problem it that it is the only only parameter that does break the symmetry between the options and properties: maybe there was a special reason for this ?
options.cwd
-> file.cwd
options.base
-> file.base
options.path
-> file.path
options.history
-> file.history
options.stat
-> ???options.contents
-> file.contents
Since you confirm that stat
is a valid property, I'll submit a PR adding it to the properties list.
@demurgos stat
is not a valid property (insomuch as a custom property tacked on). The documented properties are documented because they are getters/setters and stat
is not.
It may become a getter/setter property in the future (once https://github.com/gulpjs/vinyl/issues/105 is tackled) but is not currently and should not be documented as such.
I understand that it does not have any special getter or setter, but I think that the documentation should mention it anyway.
As a user I do not need to know if it is backed by a getter or if it is a simple value: I want to know if I can do var foo = file.stat
or not.
Since it is a "normal" value (not a getter/setter), I'd just describe what it means (as in options
) and once you implement #105, update the documentation to specify what special behaviour was added. For the moment, I'd simply write:
#### `file.stat`
The result of an `fs.stat` call. This is how the file is marked as a directory
or symbolic link. See [isDirectory()][is-directory],
[isSymbolic()][is-symbolic] and [fs.Stats][fs-stats] for more information.
Type: [`fs.Stats`][fs-stats]
The reason why I care about that is that I contribute to the definitions of vinyl
: what is available does matter. (And even if it's written that the options are copied, is not that obvious if you just want to jump to the "properties" part of the documentation)
That's incorrect though. The property doesn't exist if you don't pass it in the options. It's more of an internal property once passed as options. That's why I don't think it belongs as documented as a property. Bad things can happen if a user changes things incorrectly.
The property doesn't exist if you don't pass it in the options.
I would argue that it exists more than some other properties that made it to the documentation:
These two lines ensure that the key is there, and it is not a mere undefined
but a null
: it exists but does not have any value.
If I run a small REPL session, I get this:
$ node
> var Vinyl = require("vinyl");
var Vinyl = require("vinyl");
undefined
> var file = new Vinyl();
undefined
> Object.keys(file);
[ 'history', 'cwd', 'base', 'stat', '_contents', '_isVinyl' ]
> file.contents
null
> file.stat
null
> file.notAProperty
undefined
Both contents
and stat
are null
, and what's worse: if I iterate on the keys I get stat
but not contents
.
If the null
value still bothers you, what do you think about this formulation ?
file.stat
The result of an
fs.stat
call. This is how the file is marked as a directory or symbolic link. See [isDirectory()][is-directory], [isSymbolic()][is-symbolic] and [fs.Stats][fs-stats] for more information.Note that its value can be
null
if thestat
option was never defined.Type: [
fs.Stats
][fs-stats] | null
.
Bad things can happen if a user changes things incorrectly.
I strongly agree on this, but even if you currently do not enforce any checks with a setter, it's better to say that "this property is for stats that are instances of fs.Stat
" than to not mention it.
I think the documentation you are proposing is incorrect. By stating "the result of an fs.stat
call", it is making it seem as though we make an fs.stat
call upon the path you pass in. However, I think the header "Instance properties" is also incorrect in the current docs and it should be "Instance property accessors".
Stating that you iterate over a property doesn't make for a convincing argument, as you also iterate over our _isVinyl
property which is also used internally.
Stating that you iterate over a property doesn't make for a convincing argument, as you also iterate over our
_isVinyl
property which is also used internally.
Ok, iterating over the keys does not make much sense because of the internal properties. Here is another snippet exploiting the information from Object.keys
:
var Vinyl = require("vinyl");
var file = new Vinyl();
if ("stat" in file) { // This will always be true
console.log("Stat:", file.stat);
}
Even if it is used directly here, you can easily imagine it in a function in an other file that inspects it's arguments.
I think the documentation you are proposing is incorrect. By stating "the result of an
fs.stat
call", it is making it seem as though we make anfs.stat
call upon the path you pass in.
You are right, the above formulation was misleading. Here is an other try:
file.stat
An [
fs.Stats
][fs-stats] instance describing the Vinyl object. This is used to mark the Vinyl object as a directory or a symbolic link. See [isDirectory()][is-directory], [isSymbolic()][is-symbolic] and [fs.Stats][fs-stats] for more information.Note: it is
null
by default, you have to either pass it in thestat
option of the constructor or set it manually.Type: [
fs.Stats
][fs-stats] |null
However, I think the header "Instance properties" is also incorrect in the current docs and it should be "Instance property accessors".
I think that the current wording is expected and correct according to the spec:
4.3.30 property
part of an object that associates a key (either a String value or a Symbol value) and a value Note
Note: Depending upon the form of the property the value may be represented either directly as a data value (a primitive value, an object, or a function object) or indirectly by a pair of accessor functions.
The note clarifies that a property can be either direct or indirect, but it is still a property.
Hi,
file.isDirectory()
mentions thefile.stat
property.The property
file.stat
is not documented. Could you clarify this ?