Skip to content

.exit message #3368

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed

.exit message #3368

wants to merge 1 commit into from

Conversation

hemanth
Copy link
Contributor

@hemanth hemanth commented Oct 14, 2015

^

@mscdex mscdex added the repl Issues and PRs related to the REPL subsystem. label Oct 14, 2015
@pselle
Copy link

pselle commented Oct 14, 2015

I would put that in the same parentheses as the ^C message, otherwise I 👍 this change!

@jasnell
Copy link
Member

jasnell commented Oct 14, 2015

Needs a proper commit message and PR description. Please see https://github.com/nodejs/node/blob/master/CONTRIBUTING.md

@silverwind
Copy link
Contributor

-1, I think the current message is sufficient. It makes no sense to input .exit after a CTRL-C, when one can just .exit without the preceding keypress.

@hemanth
Copy link
Contributor Author

hemanth commented Oct 14, 2015

@jasnell, sorry missed that, OK with the branch name or shall I do a fresh one?

@pselle I was in a dilama :\

@jasnell
Copy link
Member

jasnell commented Oct 14, 2015

@hemanth .. the branch name is inconsequential. Please note @silverwind's objection, however.

@pselle
Copy link

pselle commented Oct 14, 2015

So this is a usability/docs note. While it makes sense to use ^C after used once, the idea is to educate the user that .exit is also an exit option

@RobAtticus
Copy link

I did learn from this the existence of .exit, so as a user I think this would be helpful.

@ronkorving
Copy link
Contributor

Wow, I learned something today. But I would rephrase "use" to something like "type". The first impression I had when reading "use .exit" was that it would mean "use the process.exit method".

I am however completely missing the point of the existence of .exit. What's the use case? My fingers are already on the CTRL-C buttons, why would I start typing ".exit" instead?

@thefourtheye
Copy link
Contributor

I second @silverwind here. Normally when I press Ctrl + C (or Ctrl + D), I expect the program to terminate. But, if it doesn't then I would expect some valid message being shown to me. In this case, the message being printed looks perfectly fine to me.

@Fishrock123
Copy link
Contributor

Considering this is only on an interrupt, I'm not sure if it makes sense?

@hemanth
Copy link
Contributor Author

hemanth commented Oct 15, 2015

the idea is to educate the user that .exit is also an exit option

^ +1

Considering this is only on an interrupt, I'm not sure if it makes sense?

Not sure

It makes no sense to input .exit after a CTRL-C

Agree!

@jasnell
Copy link
Member

jasnell commented Oct 15, 2015

@hemanth ... this is a good step, but I think something like the following would be better:

self.output.write('(To exit, press ^C again or type .exit)\n');

@pselle
Copy link

pselle commented Oct 15, 2015

+1 to @jasnell 's wording.

@jasnell
Copy link
Member

jasnell commented Nov 5, 2015

@hemanth ... would you be able/willing to update the PR with the suggested wording in #3368 (comment)?

@hemanth
Copy link
Contributor Author

hemanth commented Nov 6, 2015

@jasnell Done.

@jasnell
Copy link
Member

jasnell commented Nov 6, 2015

LGTM!

@jasnell
Copy link
Member

jasnell commented Nov 6, 2015

Oh... one last thing... can you squash the commits and fix up the commit log to follow the standard format? https://github.com/nodejs/node/blob/master/CONTRIBUTING.md#step-3-commit

When the user hits `^C` in the REPL show more info about `.exit`.

The idea was to give more info to the user when they hit ^C.
Current version just displays `(^C again to quit)` and most
of the users are not aware of the `.exit` command that would
Exit the repl.
@hemanth
Copy link
Contributor Author

hemanth commented Nov 7, 2015

@jasnell Looks fine now?

@jasnell
Copy link
Member

jasnell commented Nov 7, 2015

Yes perfect. Thank you!

jasnell pushed a commit that referenced this pull request Nov 9, 2015
When the user hits `^C` in the REPL show more info about `.exit`.

The idea was to give more info to the user when they hit ^C.
Current version just displays `(^C again to quit)` and most
of the users are not aware of the `.exit` command that would
Exit the repl.

PR-URL: #3368
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 9, 2015

Landed in 354f9ce

@hemanth
Copy link
Contributor Author

hemanth commented Nov 10, 2015

👍 Thanks!

Fishrock123 pushed a commit that referenced this pull request Nov 10, 2015
When the user hits `^C` in the REPL show more info about `.exit`.

The idea was to give more info to the user when they hit ^C.
Current version just displays `(^C again to quit)` and most
of the users are not aware of the `.exit` command that would
Exit the repl.

PR-URL: #3368
Reviewed-By: James M Snell <jasnell@gmail.com>
This was referenced Nov 10, 2015
MylesBorins pushed a commit that referenced this pull request Nov 16, 2015
When the user hits `^C` in the REPL show more info about `.exit`.

The idea was to give more info to the user when they hit ^C.
Current version just displays `(^C again to quit)` and most
of the users are not aware of the `.exit` command that would
Exit the repl.

PR-URL: #3368
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

landed in lts-v4.x-staging as bf48969

@hemanth
Copy link
Contributor Author

hemanth commented Nov 17, 2015

👍 Thanks.

rvagg pushed a commit that referenced this pull request Dec 4, 2015
When the user hits `^C` in the REPL show more info about `.exit`.

The idea was to give more info to the user when they hit ^C.
Current version just displays `(^C again to quit)` and most
of the users are not aware of the `.exit` command that would
Exit the repl.

PR-URL: #3368
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
When the user hits `^C` in the REPL show more info about `.exit`.

The idea was to give more info to the user when they hit ^C.
Current version just displays `(^C again to quit)` and most
of the users are not aware of the `.exit` command that would
Exit the repl.

PR-URL: #3368
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
When the user hits `^C` in the REPL show more info about `.exit`.

The idea was to give more info to the user when they hit ^C.
Current version just displays `(^C again to quit)` and most
of the users are not aware of the `.exit` command that would
Exit the repl.

PR-URL: #3368
Reviewed-By: James M Snell <jasnell@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants