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
Update doc comments and regenerate README.md. #825
Conversation
Follow-up to #824. :) |
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.
Terms used as improper nouns don't need backticks (e.g., "This command operates over each file" instead of "This command operates over each file
"). I commented on some of these, but not every instance.
Everything else looks mostly good, I can give another pass later.
README.md
Outdated
@@ -151,7 +151,7 @@ var str = cat('file1', 'file2'); | |||
var str = cat(['file1', 'file2']); // same as above | |||
``` | |||
|
|||
Returns a string containing the given file, or a concatenated string | |||
Returns a string containing the given `file`, or a concatenated string |
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.
No backticks on "file" (it's a noun, not a variable)
README.md
Outdated
Available options: | ||
|
||
+ `-v`: output a diagnostic for every file processed | ||
+ `-c`: like verbose but report only when a change is made | ||
+ `-v`: output a diagnostic for every `file` processed |
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.
Same as above
README.md
Outdated
|
||
### popd([options,] ['-N' | '+N']) |
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.
Did we change the blank lines around this line?
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.
There was 1 less blank line above this than the other headings, so I made it match.
README.md
Outdated
@@ -213,15 +212,14 @@ Copies files. | |||
|
|||
|
|||
### pushd([options,] [dir | '-N' | '+N']) | |||
|
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.
Missing blank line?
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 noticed there were sometimes blank lines after a heading, and sometimes not. I looked at all of them and it seemed like the more common case was no blank line, so I made it match.
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.
Blank line is probably more appropriate, actually. Markdown is weird, but "regular line followed immediately by a bullet" doesn't work, while "regular line followed by a blank line and then by a bullet" does work.
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.
Oh, we don't even have a bullet here. Point still stands though:
regular line
regular line
render as: "regular line regular line"
while:
## header line
regular line
renders as:
header line
regular line
I usually add a blank line in-between for consistency with the regular-line case.
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! I’ll add a blank line after each header.
README.md
Outdated
|
||
Not seeing the behavior you want? `exec()` runs everything through `sh` | ||
by default (or `cmd.exe` on Windows), which differs from `bash`. If you | ||
need bash-specific behavior, try out the `{shell: 'path/to/bash'}` option. | ||
|
||
**Note:** For long-lived processes, it's best to run `exec()` asynchronously as | ||
**Note:** For long-lived processes, it's best to run `exec()` asynchronously, as |
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.
Actually, you can just remove this. The CPU concerns are no longer relevant.
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.
Will do!
README.md
Outdated
file that match the given `regex_filter`. | ||
|
||
|
||
### head([{'-n': \<num\>},] file [, file ...]) | ||
### head([{'-n': \<num\>},] file_array) | ||
Available options: | ||
|
||
+ `-n <num>`: Show the first `<num>` lines of the files | ||
+ `-n <num>`: Show the first `<num>` lines of the `file`s |
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.
No backticks
README.md
Outdated
@@ -384,7 +382,7 @@ var str = head('file1', 'file2'); | |||
var str = head(['file1', 'file2']); // same as above | |||
``` | |||
|
|||
Read the start of a file. | |||
Read the start of a `file`. |
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.
No backticks
README.md
Outdated
|
||
|
||
### mkdir([options,] dir [, dir ...]) | ||
### mkdir([options,] dir_array) | ||
Available options: | ||
|
||
+ `-p`: full path (will create intermediate dirs if necessary) | ||
+ `-p`: full path (will create intermediate `dir`s, if necessary) |
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 change this to "directories" (no backticks)
README.md
Outdated
This is a partial implementation of *[touch(1)](http://linux.die.net/man/1/touch)*. | ||
Update the access and modification times of each `FILE` to the current time. | ||
A `FILE` argument that does not exist is created empty, unless `-c` is supplied. | ||
This is a partial implementation of *[`touch(1)`](http://linux.die.net/man/1/touch)*. |
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.
Let's remove italics
README.md
Outdated
|
||
**Note**: do not rely on the | ||
**Note**: Do not rely on the |
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.
No capital
Also, it looks like you need to re-run |
Codecov Report
@@ Coverage Diff @@
## master #825 +/- ##
=======================================
Coverage 95.49% 95.49%
=======================================
Files 34 34
Lines 1266 1266
=======================================
Hits 1209 1209
Misses 57 57
Continue to review full report at Codecov.
|
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.
Thanks for cleaning this up, this looks a lot better!
@freitagbr feel free to chime in, otherwise I'll merge.
Thanks, I edited commit message. |
No description provided.