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

Made maxOutput Prop #1015

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

BjornTheProgrammer
Copy link

I noticed while using react-console-emulator, that the terminal can get very laggy when many high-speed terminal.pushToStdout() occur, and it gets much worse when the terminal output begins to be the length of millions. Implementing a solution using refs would be painful, and I think it would be much more reasonable to add the option for other people to use, as it is only one line of code.

Copy link
Owner

@linuswillner linuswillner left a comment

Choose a reason for hiding this comment

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

Just a few small nits, otherwise good.

(Historical note: This library is indeed probably a potent candidate to have performance problems, given how old some of the code is. It should be rewritten in TypeScript and all that jazz, but time is finite.)

src/Terminal.jsx Outdated Show resolved Hide resolved
docs/CONFIG.md Outdated Show resolved Hide resolved
@BjornTheProgrammer
Copy link
Author

Sorry for spamming commits, I just kept remembering that I could make things a bit better.

@linuswillner
Copy link
Owner

No problem, sorry for the delay - I don't really check in on this repo that often. I had one more suggestion but otherwise I think this is good.

@BjornTheProgrammer
Copy link
Author

No problem, sorry for the delay - I don't really check in on this repo that often. I had one more suggestion but otherwise I think this is good.

Don't worry about the delay, I know you're busy and that you provided this project in your free time! Thank you for all the work you've put into refining this feature. Appreciate the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants