Break up arguments section#2586
Conversation
docs/arguments.rst
Outdated
| Like options, arguments can also grab values from an environment variable. | ||
| Unlike options, however, this is only supported for explicitly named | ||
| environment variables. | ||
| This feature is not recommended since it be confusing to users. Arguments can only pull environment variables from ? explicitly named environment variables. In that case, it can also be a list of different environment variables where the first one is picked. ? |
There was a problem hiding this comment.
@AndreasBackx Do you know what the documentation could mean in this section? I have not a clue so providing clearer wording and a better example is difficult.
There was a problem hiding this comment.
I'm not sure myself, I'd have to look in the code. I'd have a look around the implementation or look at the blame of that documentation, maybe the commit/PR introducing it has some history.
There was a problem hiding this comment.
@AndreasBackx I think actually only Armin knew. It was basically unchanged since the first commit. But I figured it out by playing around. Could you give this PR a once over?
517df5a to
424d0e7
Compare
AndreasBackx
left a comment
There was a problem hiding this comment.
What I really like about these changes is that some of this new documentation is so much more easily skimmable. Instead of rather large paragaphs, we have easy to understand lists. I am a big fan of this and we should to this elsewhere in the documentation as well!
Most of my comments are improvements towards:
- Consistency in writing.
- Adding typing which aids with what we're documenting here.
- Some regressions I think we're making somewhere.
docs/handling-files.rst
Outdated
| File open behavior can be controlled by the boolean kwarg ``lazy``. If a file is opened lazily: | ||
|
|
||
| * A failure at first IO operation will happen by raising an :exc:`FileError`. | ||
| * It can help minimize resource handling confusion. If a file is opened in lazy mode, it will receive a ``close_intelligently`` method that can help figure out if the file needs closing or not. This is not needed for parameters, but is necessary for manually prompting. For manual prompts with the :func:`prompt` function you do not know if a stream like stdout was opened (which was already open before) or a real file was opened (that needs closing). |
There was a problem hiding this comment.
Do we have a page anywhere where close_intelligently is documented? Would be nice to link to it because this does not explain that enough imho.
There was a problem hiding this comment.
I agree it does not explain enough. It seems like it refers to specific parts of the code but I am not sure which.
There was a problem hiding this comment.
It refers to LazyFile.close_intelligently. Could you add that reference? That should be good for now. LazyFile is in utils.
429d5d4 to
e4575dd
Compare
e4575dd to
9959aa9
Compare
|
Was not able to include full type hints because of issues with custom sphinx directive we use to run the code. |
Break up the arguments section into arguments and handling files.