Conversation
olleolleolle
left a comment
olleolleolle
left a comment
There was a problem hiding this comment.
👏 Thanks, @BurdetteLamar for working on the readability of this documentation.
| # Note that an option type has to do with the <em>option argument</em> | ||
| # (whether it is required, optional, or forbidden), | ||
| # not with whether the option itself is required. |
lib/getoptlong.rb
Outdated
| # | ||
| # ==== Option with Optional Argument | ||
| # | ||
| # An option of type <tt>GetoptLong::OPTIONAL_ARGUMENT</tt>: |
There was a problem hiding this comment.
| # An option of type <tt>GetoptLong::OPTIONAL_ARGUMENT</tt>: | |
| # An option of type <tt>GetoptLong::OPTIONAL_ARGUMENT</tt> |
(Same as above, perhaps the colon is unnecessary.)
| # ["--xxx", "Foo"] | ||
| # ["--yyy", "Bar"] | ||
| # ["--zzz", ""] | ||
| # Remaining ARGV: ["Baz", "Bat", "Bam"] |
And thanks, @olleolleolle, for your careful review. |
|
@hsbt, is this ready for review and/or merge? |
|
I'm not native. I couldn't review this. |
lib/getoptlong.rb
Outdated
| # ["--xxx", ""] | ||
| # ["--xyz", ""] | ||
| # | ||
| # === Protected Options |
There was a problem hiding this comment.
Seems weird to call this "protected options"? Does GetoptLong use that wording internally? -- on the command line indicates that all following entries in argv are arguments and not options, even if they start with a dash.
There was a problem hiding this comment.
I agree that "protected options" is weak, and is not used internally. I don't have a better term to use as the heading.
There was a problem hiding this comment.
Maybe "Treating Remaining Options as Arguments"?
lib/getoptlong.rb
Outdated
| # pure Ruby implementation. | ||
| # A simple example: file <tt>simple.rb</tt>: | ||
| # | ||
| # :include: ../doc/simple.rb |
There was a problem hiding this comment.
How does this including work for usage as a default gem in Ruby? I'm guessing it wouldn't work, since we don't want to have simple.rb in doc in the Ruby repository. Possibly we could use a doc/getoptlong directory instead. Not sure if that is desired. Hopefully @hsbt can provide input.
There was a problem hiding this comment.
Will wait for direction on this one.
There was a problem hiding this comment.
Have moved included files to doc/getoptlong, which is consistent with other default gems' usage. Also (the horror!), I did not realize that directory doc was in the .gitignore file, and so only now are the .rdoc files getting into this PR.
|
I see that there's no maintainer for this repo. So how does this (or anything) get reviewed and merged? |
jeremyevans
left a comment
jeremyevans
left a comment
There was a problem hiding this comment.
I think this is a major improvement to the documentation, I just have a couple of requests for changes.
lib/getoptlong.rb
Outdated
| # === ARGV | ||
| # | ||
| # \GetoptLong does not process the command line itself, | ||
| # but instead processes array ARGV (which is derived from the command line). |
There was a problem hiding this comment.
I would remove discussion of command lines. Outside of Windows, programs in general are called with array of arguments, not with a command line. If you are executing commands from the shell, the shell will take the command line, parse it, and call the program with an array of arguments. You can focus discussion on ARGV, without discussing command lines.
There was a problem hiding this comment.
Do you mean just here? Or throughout (28 mentions of command line, including example command lines)?
There was a problem hiding this comment.
When discussing example command lines, it's fine to keep the reference. However, any place you are discussing argument processing, the reference should be removed. It's important to understand that the command line to argument processing is unrelated to option parsing. Option parsing via optparse/getoptlong is for taking an array of arguments and separating out the options. From a lexing/parsing perspective, the shell lexes, and optparse/getoptlong parses.
| /.yardoc | ||
| /_yardoc/ | ||
| /coverage/ | ||
| /doc/ |
There was a problem hiding this comment.
/doc/ may be ignored for a reason. Maybe because it is the default location for rdoc to generate documentation? I recommend having the example code in an examples directory, as that better indicates that these are executable examples. You can still include the code in the documentation with :include:
There was a problem hiding this comment.
Moved to examples directory.
|
@jeremyevans, ready for review. |
jeremyevans
left a comment
jeremyevans
left a comment
There was a problem hiding this comment.
examples change looks fine. I still think it would be a good idea to change the command line references, but I'll leave that to you. getoptlong is currently unmaintained, so you can merge this yourself after you decide whether to change the references to the command line.
@jeremyevans, I'm happy to make these further changes. I've removed a dozen referenced to the command line, leaving only those that refer to an actual example command line. Hope I haven't removed too many. |
Detailed introductory material. ruby/getoptlong@1544f2fb7b
Detailed introductory material. ruby/getoptlong@1544f2fb7b
Detailed introductory material. ruby/getoptlong@1544f2fb7b
4 participants
Enhanced RDoc for GetoptLong.