-
Notifications
You must be signed in to change notification settings - Fork 498
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 mod.rs changes. #450
Conversation
src/items/modules.md
Outdated
mod local_data; | ||
} | ||
``` | ||
> **Note**: Module filenames may also be the name of the module as a directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That util/mod.rs
is also valid is a part of the language, and not just a note. The previous to Rust 1.30 should stay as a note (though I'd rather it say rustc
, since I don't think Rust is versioned right now). We should also explicitly say it is deprecated.
Also, is this allowed?
#[path = "util.rs"] mod util_1;
#[path = "util/mod.rs"] mod util_2;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll move it out of the note, though that may complicate it some.
We should also explicitly say it is deprecated.
I wouldn't say it is deprecated. The RFC explicitly says "The use of mod.rs continues to be allowed without any deprecation." To me, deprecated usually implies a lint that triggers, but in this case not even the edition-idiom lints recommend this. I carefully chose to say it is "encouraged" though I don't think anyone has officially established even that.
Also, is this allowed?
Haha, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. Scratch the deprecation part then.
Though, given the interaction exists, I'm thinking it might it make sense to describe the chosen file as an algorithm? I want to say probably not, since we can statically state that the path attribute changes the resolution. But in that case, we should state that this is only the case when the path attribute is not applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is only the case when the path attribute is not applied.
Yea, I tried to imply that by starting with "By default", but that is a little too subtle. I'll try to think of a way to make it clear the distinction between "default" behavior and overridden behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the only non-default is when there's a path attribute, you can explicitly say "When the module has no path attribute"
Note: I'm still waiting for the resolution for rust-lang/rust#55560 and rust-lang/rust#55094 before finishing this, since things are still in flux. |
Clarifying: the current implemented behavior is that using |
I have pushed a new update. I tried to balance being precise and clear with being too verbose. I don't feel I hit the target, though. It's very difficult to describe path attributes succinctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few style nits
Thanks! |
Stabilization PR: rust-lang/rust#54072
Tracking issue: rust-lang/rust#53125
Part of RFC 2126.