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

Force process.stdout to be a TTY to support spinners #1163

Closed
wants to merge 4 commits into from

Conversation

iiroj
Copy link
Member

@iiroj iiroj commented Jun 1, 2022

No description provided.

@iiroj iiroj changed the base branch from master to split-integration-tests June 1, 2022 13:29
lib/forceTty.js Outdated Show resolved Hide resolved
@iiroj iiroj force-pushed the fix-renderer branch 7 times, most recently from bcb730b to 1e83625 Compare June 1, 2022 15:03
@codecov
Copy link

codecov bot commented Jun 1, 2022

Codecov Report

Merging #1163 (8797a9c) into master (32806da) will decrease coverage by 3.05%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##            master    #1163      +/-   ##
===========================================
- Coverage   100.00%   96.94%   -3.06%     
===========================================
  Files           25       26       +1     
  Lines          729      752      +23     
  Branches       197      201       +4     
===========================================
  Hits           729      729              
- Misses           0       20      +20     
- Partials         0        3       +3     
Impacted Files Coverage Δ
lib/forceTty.js 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32806da...8797a9c. Read the comment docs.

@iiroj iiroj force-pushed the fix-renderer branch 2 times, most recently from 32fcd92 to db6618c Compare June 1, 2022 16:06
Base automatically changed from split-integration-tests to master June 8, 2022 17:14
@dennisjlee
Copy link

I tested this PR on Windows and I made several changes in order to make it work: dennisjlee@98d1dbf

@iiroj should I send a PR from my fork?

@iiroj
Copy link
Member Author

iiroj commented Jun 28, 2022

@dennisjlee yeah, please do! Try to keep the code async if possible, though. I see you replaced some methods with the sync versions.

@dennisjlee
Copy link

@iiroj I tried to keep it async but I ran into some blockers - I'll open the PR and explain there

@dennisjlee
Copy link

I sent #1181 as a PR targeting this fix-renderer branch

@Ronsku
Copy link

Ronsku commented Nov 5, 2022

Is this being worked on? This is kind of blocking me from using lint-staged. I appreciate the work you have put down into this! 🌟

Here are some results and info:

For me it runs crazy many times, this list about 5-10 times longer than what I've screenshot here.
image

@iiroj
Copy link
Member Author

iiroj commented Nov 6, 2022

Hello, the problem was caused by git and fixed (at least for me) by a later git update.

Since I don't have a Windows environment, I haven't put more effort into this.

@dennisjlee
Copy link

Is this being worked on? This is kind of blocking me from using lint-staged. I appreciate the work you have put down into this! 🌟

Here are some results and info:

For me it runs crazy many times, this list about 5-10 times longer than what I've screenshot here. image

I had a pending modification to this PR in #1181 but unfortunately I haven't been able to revisit this for a long time.
However, I did try upgrading my Git for Windows to the latest (v2.38.1, from v2.33.0) and that did not resolve the issue for me.

@iiroj if I have time to revisit #1181, would you want me to still land that on top of your fix-renderer branch, or would it make sense to combine #1181 and #1163 into a separate PR?

@iiroj
Copy link
Member Author

iiroj commented Nov 6, 2022

@dennisjlee feel free to try out whatever. 👍 Keeping a single PR open is easiest since I have to manually trigger pipelines.

@Ronsku
Copy link

Ronsku commented Nov 7, 2022

Thank you @dennisjlee! I appreaciate it a lot! 😊

@stec00
Copy link

stec00 commented Sep 4, 2023

Is this PR still being worked on?

@iiroj
Copy link
Member Author

iiroj commented Oct 14, 2023

Closing as the original issue should be already be fixed in more recent git versions.

@iiroj iiroj closed this Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

vscode shows all the process logs in the termimal, how to dismiss them?
5 participants