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

Add flag to disable terminal clear in --watch mode #45713

Closed
silverwind opened this issue Dec 2, 2022 · 5 comments · Fixed by #45717
Closed

Add flag to disable terminal clear in --watch mode #45713

silverwind opened this issue Dec 2, 2022 · 5 comments · Fixed by #45717
Labels
feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. watch-mode Issues and PRs related to watch mode

Comments

@silverwind
Copy link
Contributor

silverwind commented Dec 2, 2022

What is the problem this feature will solve?

Currently --watch clears the terminal every time it restarts the process. This can hide vital information that the process logs on shutdown or from other processes running in the same terminal.

What is the feature you are proposing to solve the problem?

Remove the terminal clearing because it is opinionated. If this is not desired, make it opt-out via a flag like TypeScript's preserveWatchOutput or the nodemon solution.

What alternatives have you considered?

nodemon also does not clear terminal on restart, so it is an alternative.

@silverwind silverwind added the feature request Issues that request new features to be added to Node.js. label Dec 2, 2022
@MoLow MoLow added the watch-mode Issues and PRs related to watch mode label Dec 2, 2022
@MoLow
Copy link
Member

MoLow commented Dec 2, 2022

sounds like a flag/config that can be added 👍🏻

@MoLow MoLow added the good first issue Issues that are suitable for first-time contributors. label Dec 2, 2022
@silverwind silverwind changed the title --watch should not clear terminal Add flag to disable terminal clear in --watch mode Dec 2, 2022
@silverwind
Copy link
Contributor Author

silverwind commented Dec 2, 2022

Thanks, updated issue title and I also investigated nodemon, they default to not clearing but there are various supported ways to achieve clearing. I think it should be definitely made configurable because user preferences vary.

@debadree25
Copy link
Member

hello, how taking a stab at the issue, how do I whitelist a command/flag
I am getting this error message out/Release/node: bad option: --dont-clear-on-restart any pointers would be very much appreciated

Thank you!
cc @MoLow

debadree25 added a commit to debadree25/node that referenced this issue Dec 2, 2022
Added a --preserve-output flag to watch mode
Fixes: nodejs#45713
@debadree25
Copy link
Member

Ok was able to figure out have started a draft PR, attempting to add tests

@tniessen
Copy link
Member

tniessen commented Dec 2, 2022

Quoting myself from #45717:

Is clearing the terminal really a good default choice? Doesn't that potentially take away error messages etc., depending on the terminal host? What if stdout is not a TTY?

debadree25 added a commit to debadree25/node that referenced this issue Dec 11, 2022
Added a --watch-preserve-output flag to watch mode
Fixes: nodejs#45713
aduh95 pushed a commit that referenced this issue Dec 11, 2022
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
ErickWendel pushed a commit to ErickWendel/node that referenced this issue Dec 12, 2022
Fixes: nodejs#45713
PR-URL: nodejs#45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
targos pushed a commit that referenced this issue Dec 12, 2022
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
targos pushed a commit that referenced this issue Dec 13, 2022
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
danielleadams pushed a commit that referenced this issue Dec 30, 2022
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
danielleadams pushed a commit that referenced this issue Jan 3, 2023
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
danielleadams pushed a commit that referenced this issue Jan 4, 2023
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
danielleadams pushed a commit that referenced this issue Jan 5, 2023
Fixes: #45713
PR-URL: #45717
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. good first issue Issues that are suitable for first-time contributors. watch-mode Issues and PRs related to watch mode
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants