Skip to content
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

doc: fix an unclear wording in readline.md #12605

Closed
wants to merge 1 commit into from
Closed

doc: fix an unclear wording in readline.md #12605

wants to merge 1 commit into from

Conversation

vsemozhetbyt
Copy link
Contributor

Checklist
Affected core subsystem(s)

doc, readline

As a not native speaker, I am not sure if this is really a typo. If not, sorry for the bothering)

@vsemozhetbyt vsemozhetbyt added doc Issues and PRs related to the documentations. readline Issues and PRs related to the built-in readline module. labels Apr 23, 2017
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. readline Issues and PRs related to the built-in readline module. labels Apr 23, 2017
@@ -403,8 +403,8 @@ a `'resize'` event on the `output` if or when the columns ever change

### Use of the `completer` Function

When called, the `completer` function is provided the current line entered by
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just provided -> passed?

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt Apr 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without a preposition? 'When called, the completer function is passed the current line...'? I am a bit confused by this passive voice. Can we say 'Provide the function a line' without a preposition? It seems we can say 'Pass the function a line', so the passive can be without a preposition here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, passed is transitive so works.

When called, the completer function is provided the current line entered by the user === When the completer function is called (by Node.js), the current line entered by the user is passed to it.

I know we normally prefer the active voice, nothing against using that here, the only issue is that AIUI this is a callback function, so it is called by the Node.js readline code, the arguments are passed into it, and Node expects it to return an Array. So you can't say Provide the function a line, because the user doesn't do that, Node does.

You could maybe say something like:

The completer function takes the current line entered by the user as an argument, and returns an Array with 2 entries:

I defer to you on what's clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I shall use your last variant and let's see if anybody come along with us)

@vsemozhetbyt vsemozhetbyt changed the title doc: fix typo in readline.md doc: fix an unclear wording in readline.md Apr 23, 2017
@vsemozhetbyt
Copy link
Contributor Author

Landed in 2098775

@vsemozhetbyt vsemozhetbyt deleted the readline.md-typo branch April 25, 2017 19:13
vsemozhetbyt added a commit that referenced this pull request Apr 25, 2017
PR-URL: #12605
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@evanlucas evanlucas mentioned this pull request May 1, 2017
evanlucas pushed a commit that referenced this pull request May 1, 2017
PR-URL: #12605
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas pushed a commit that referenced this pull request May 2, 2017
PR-URL: #12605
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this pull request May 16, 2017
PR-URL: #12605
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request May 18, 2017
PR-URL: #12605
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request May 23, 2017
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
PR-URL: nodejs/node#12605
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. readline Issues and PRs related to the built-in readline module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants