GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: clarify examples section in REPL doc by dario-piotrowicz · Pull Request #57762 · nodejs/node

Conversation

@dario-piotrowicz

@dario-piotrowicz dario-piotrowicz commented Apr 5, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This is pretty minor, but I just figured a little bit of polishing wouldn't hurt 🙂

The current examples presented at the bottom of the REPL doc have three issues which I am trying to address here:

  • they look like they're part of the section above (on how to run multiple REPL instances in the same process) but they are not

    see the table of contents at the top of the REPL page:

    before after
    before-1 (the examples are under the multiple REPL section) after-1 (the examples are listed on their own)

  • they link to old gists with code that uses outdates JS syntax (e.g var) and deprecated utilities (e.g. new Buffer)
    (so not something really ideal to present as an official example)

  • the alert informing readers not to use the second example in production environments can be wrongly interpreted as to refer to both examples

    (this might be just me, but when I read the text there it really looked to me like the warning was referring to both examples and that This example was likely a typo and that These examples was supposed to be used)

    before after
    before the warning is ananbiguously in the section specific for the example

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to Node.js documentation. repl Issues and PRs related to the REPL subsystem. labels Apr 5, 2025
@dario-piotrowicz
dario-piotrowicz force-pushed the dario/doc/repl-examples-format branch from bb2fea9 to 1edd98a Compare April 5, 2025 18:33
aduh95
aduh95 reviewed Apr 6, 2025
View reviewed changes
Comment thread doc/api/repl.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Apr 6, 2025
View reviewed changes
Comment thread doc/api/repl.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Apr 6, 2025
View reviewed changes
Comment thread doc/api/repl.md Outdated

@aduh95 aduh95 Apr 6, 2025

Copy link
Copy Markdown
Contributor

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 wonder if it still makes sense to link to an external gist, maybe we should copy the code over (not sure how it's licensed though it seems short enough to not fall under copyrightable code).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@dario-piotrowicz dario-piotrowicz Apr 6, 2025

Copy link
Copy Markdown
Member Author

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 totally agree, an external gist to a personal github user feels off to me 😕

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@jasnell jasnell Apr 6, 2025

Copy link
Copy Markdown
Member

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.

IMO, We should create our own example derived from the sample and still link over to the gist for credit.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@dario-piotrowicz dario-piotrowicz Apr 6, 2025

Copy link
Copy Markdown
Member Author

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 can definitely do that 🙂👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@dario-piotrowicz dario-piotrowicz Apr 6, 2025

Copy link
Copy Markdown
Member Author

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.

done 🙂 0c1abbe

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@dario-piotrowicz dario-piotrowicz Apr 6, 2025

Copy link
Copy Markdown
Member Author

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.

Ah yeah, one last question! 🙂

Do you think that the multi REPL section should also be moved under the new examples section? 🙂

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
dario-piotrowicz and others added 3 commits April 6, 2025 20:10
@dario-piotrowicz
doc: clarify examples section in REPL doc
2ae7045 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues
@dario-piotrowicz @aduh95
Apply suggestions from code review
85a6442 
Capitalize list items

Co-authored-by: Antoine du Hamel <[email protected]>
@dario-piotrowicz
port gist examples into docs
0c1abbe
@dario-piotrowicz
dario-piotrowicz force-pushed the dario/doc/repl-examples-format branch from 29b0720 to 0c1abbe Compare April 6, 2025 20:25
jasnell
jasnell approved these changes Apr 6, 2025
View reviewed changes
aduh95
aduh95 reviewed Apr 6, 2025
View reviewed changes
Comment thread doc/api/repl.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@dario-piotrowicz
use full URLs in gist code reference
869814b
aduh95
aduh95 reviewed Apr 6, 2025
View reviewed changes
Comment thread doc/api/repl.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Apr 6, 2025
View reviewed changes
Comment thread doc/api/repl.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Apr 6, 2025
View reviewed changes
Comment thread doc/api/repl.md Outdated
Comment on lines +1095 to +1100
// Hack to thread stdin and stdout
// simultaneously in curl's single thread
const iv = setInterval(() => {
res.write(buf0);
}, 100);

@aduh95 aduh95 Apr 6, 2025

Copy link
Copy Markdown
Contributor

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.

Is this still needed? I can't see what difference it makes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@dario-piotrowicz dario-piotrowicz Apr 6, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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 I just tried the example without it (I'm on Ubuntu) and it does not seem necessary

I wasn't actually fully sure why this was needed and feared that removing might break the example under some conditions / on some platform? 🤷

However do think that quite likely this is not needed, so I'm totally happy removing it, we can always re-add it if in some cases it is right? 🙂

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@dario-piotrowicz dario-piotrowicz Apr 7, 2025

Copy link
Copy Markdown
Member Author

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.

Removed 🙂 a977fbe

(PS: thanks so much for the suggestion 🫶)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
dario-piotrowicz and others added 2 commits April 7, 2025 00:45
@dario-piotrowicz @aduh95
Apply suggestions from code review
7f1d0f3 
Co-authored-by: Antoine du Hamel <[email protected]>
@dario-piotrowicz
remove unnecessary buffer hack
a977fbe
aduh95
aduh95 approved these changes Apr 8, 2025
View reviewed changes
@aduh95 aduh95 added author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. labels Apr 8, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Apr 8, 2025
@nodejs-github-bot
nodejs-github-bot merged commit b396991 into nodejs:main Apr 8, 2025
@nodejs-github-bot

nodejs-github-bot commented Apr 8, 2025

Copy link
Copy Markdown
Collaborator

Landed in b396991
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@dario-piotrowicz
dario-piotrowicz deleted the dario/doc/repl-examples-format branch April 8, 2025 23:01
RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@dario-piotrowicz @RafaelGSS
doc: clarify examples section in REPL doc
551d26a 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues

PR-URL: #57762
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request May 1, 2025
2025-05-06, Version 24.0.0 (Current) #57609
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@dario-piotrowicz @RafaelGSS
doc: clarify examples section in REPL doc
379718e 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues

PR-URL: #57762
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@dario-piotrowicz @aduh95
doc: clarify examples section in REPL doc
26971ab 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues

PR-URL: #57762
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@dario-piotrowicz @RafaelGSS
doc: clarify examples section in REPL doc
67976e6 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues

PR-URL: #57762
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@dario-piotrowicz @aduh95
doc: clarify examples section in REPL doc
1031fe8 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues

PR-URL: #57762
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@dario-piotrowicz @aduh95
doc: clarify examples section in REPL doc
0121388 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues

PR-URL: #57762
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@github-actions github-actions Bot mentioned this pull request May 19, 2025
2025-05-21, Version 22.16.0 'Jod' (LTS) #58388
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@dario-piotrowicz @aduh95
doc: clarify examples section in REPL doc
bde3725 
The current examples presented at the bottom
of the REPL doc have two issues:
 - they look like they're part of the section
   above (on how to run multiple REPL instances
   in the same process) but they are not
 - the alert informing readers not to use the
   second example in production environments
   can be wrongly interpreted as to refer to
   both examples
The changes here address both these issues

PR-URL: #57762
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@ghost ghost mentioned this pull request Jun 8, 2025
### Notable Changes autofix-ci/action#28
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. doc Issues and PRs related to Node.js documentation. repl Issues and PRs related to the REPL subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@dario-piotrowicz @nodejs-github-bot @jasnell @aduh95
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.