-
Notifications
You must be signed in to change notification settings - Fork 533
Module chapter rewrite #1597
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
Module chapter rewrite #1597
Conversation
Awesome, I'll take a look when I get the chance |
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.
LGTM
Thanks |
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.
Finally got around to taking a look at this. Really good stuff! I definitely understand modules better now 😄
I know this is merged already, but figured it was easiest to leave feedback in a review anyways.
## Windows Path Syntax | ||
|
||
::: important | ||
Nushell on Windows supports both forward-slashes and back-slashes as the path separator. However, to ensure that they work on all platforms, using only the forward-slash `/` in your modules is highly recommended. | ||
::: |
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.
Not sure if this should have its own header if its only content is a infobox ("importantbox"?)
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.
Yeah, not my favorite style either, but I want a header so that it shows up in the ToC and sidebar, but I think it's useful as a callout as well given its importance.
As discussed with @kubouch in #1585, this is a pretty major rewrite of the Modules Chapter, along with a few tangential updates to other chapters.
High-level
Divides Modules into two parts:
This allows newer users who just need to use first-or-third-party modules to skip all of the "Creating" part (which was the major focus in the previous version of the chapter)
Removes much of the explanation on how to import modules from the Standard Library chapter, since we can now simply point to "Using Modules". The Standard Library chapter is now much shorter as a result.
Minutiae
Modules Overview
Briefly covers what they are as well as what they can contain.
Using Modules
$env.NU_LIBS_PATH
hide
could also hide environment variables (that's nowhide-env
) that I nixed entirely.Creating Modules
Rewrote all examples to be a bit closer to (but not completely) "real world" scenarios. I hope it's okay if we bid farewell to the venerable
greetings.nu
? I believe the new examples are a bit more clear and even (mostly) more concise.Removed "inline" as an example of how to create a module. Replaced with a tip:
The discussion of
main
is important and is moved up closer to the beginning of the chapter. Thanks @132ikl for adding that in the first place. As promised in the review of that PR, I've cc'd you on this one. You should see, I believe, every concept you mentioned in this rewrite.The previous version of the chapter had this tidbit:
That just seems wrong or outdated. I've removed it. Now the section focuses on
export module
andexport use
to create submodules.Removed the redundant note about
module
withoutexport
... It was already covered above as well as in one of the examples.The previous version of the chapter used "namespace" and "prefix" frequently. As @kubouch mentioned, these are not well-defined terms in Nushell. For the most part, I tended to rewrite/reword sections with these terms.
Moved the last two examples (VirtualEnv/Conda and "Dumping Files in a Directory") to the Cookbook. They seem better aligned there.
Standard Library