-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Document file.stat
#119
Comments
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. |
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 ?
Since you confirm that |
@demurgos It may become a getter/setter property in the future (once #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. Since it is a "normal" value (not a getter/setter), I'd just describe what it means (as in
The reason why I care about that is that I contribute to the definitions of |
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. |
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 If I run a small REPL session, I get this:
Both If the
.
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 |
I think the documentation you are proposing is incorrect. By stating "the result of an Stating that you iterate over a property doesn't make for a convincing argument, as you also iterate over our |
Ok, iterating over the keys does not make much sense because of the internal properties. Here is another snippet exploiting the information from 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.
You are right, the above formulation was misleading. Here is an other try:
I think that the current wording is expected and correct according to the spec:
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 ?The text was updated successfully, but these errors were encountered: