Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

doc: add description of legacy buf.write() format #5710

Closed
wants to merge 1 commit into from

Conversation

namhyung
Copy link

The buf.write() method also supports undocumented legacy style invocation.
As the example of buf.length property uses this form, a reader can be confused
with it. Add description of these styles at the end.

@Nodejs-Jenkins
Copy link

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit namhyung/node@ef0ed42 has the following error(s):

  • Commit message line too long: 2
  • Commit message line too long: 3

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

The buf.write() method also supports undocumented legacy style
invocation.  As the example of buf.length property uses this form,
a reader can be confused with it.  Add description of these styles
at the end.
@namhyung
Copy link
Author

I updated it into my misc branch. Also changed the Contributing wiki page which I refered originally - it said 80 column is okay.

@tjfontaine
Copy link

@trevnorris were these going to go away?

@trevnorris
Copy link

That's a discussion we need to have. It's been officially deprecated in
code, but the legacy format of encoding argument next to string argument
makes more sense (following a conversation @isaacs and I had).

@@ -98,6 +98,9 @@ The method will not write partial characters.
len = buf.write('\u00bd + \u00bc = \u00be', 0);
console.log(len + " bytes: " + buf.toString('utf8', 0, len));

For some historical reason, the `encoding` can be placed at other position too
Copy link
Member

Choose a reason for hiding this comment

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

s/some historical reason/historical reasons/

@tjfontaine @trevnorris Is this eligible for merging?

Choose a reason for hiding this comment

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

@bnoordhuis I had a conversation with @isaacs about the fact that were we place the encoding argument varies throughout core. The legacy way of placing encoding actually matches better how it's done elsewhere. And the possibility of un-legacy-ifying the syntax. Isaac, your opinion?

Choose a reason for hiding this comment

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

Either way, the legacy format will actually print a deprecation warning right now, so either we remove the deprecation warning and continue to support it, or it's going to be removed in v1.0 anyways. I don't care either way really. Thoughts?

@jasnell
Copy link
Member

jasnell commented Aug 3, 2015

Closing due to lack of activity. Can revisit if someone is interested in updating the PR or if someone feels this is still relevant.

@jasnell jasnell closed this Aug 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants