Skip to content

Add example for rendering subprocess output #406

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

Merged

Conversation

kostia1359
Copy link
Contributor

No description provided.

@kostia1359 kostia1359 force-pushed the feature-add-stream-example branch 2 times, most recently from 2c7f90f to c54bc87 Compare December 19, 2020 11:39
@kostia1359 kostia1359 force-pushed the feature-add-stream-example branch from c54bc87 to e25be08 Compare December 19, 2020 11:39
@@ -0,0 +1,25 @@
'use strict';
const React = require('react');
const {render, Text, Box} = require('../../build');
Copy link
Owner

Choose a reason for hiding this comment

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

You don't have to explicitly include build folder, because it's already set as the main folder in package.json.

Suggested change
const {render, Text, Box} = require('../../build');
const {render, Text, Box} = require('../..');

Comment on lines 10 to 14
const subProcess = childProcess.spawn('ping', ['8.8.8.8']).stdout;

subProcess.on('data', newOutput => {
setOutput(newOutput.toString('ascii'));
});
Copy link
Owner

Choose a reason for hiding this comment

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

To keep it explicit, because stdout is not an instance of the process, so subProcess wouldn't be a right name for it.

Suggested change
const subProcess = childProcess.spawn('ping', ['8.8.8.8']).stdout;
subProcess.on('data', newOutput => {
setOutput(newOutput.toString('ascii'));
});
const subProcess = childProcess.spawn('ping', ['8.8.8.8']);
subProcess.stdout.on('data', newOutput => {
setOutput(newOutput.toString('ascii'));
});


return (
<Box flexDirection="column">
<Text>My ping output:</Text>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<Text>My ping output:</Text>
<Text>My <Text bold>ping</Text> command output:</Text>

return (
<Box flexDirection="column">
<Text>My ping output:</Text>
<Text>{output}</Text>
Copy link
Owner

Choose a reason for hiding this comment

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

To give it a little more breathing room. It's also probably a good idea to limit the number of lines you can print, to ensure it doesn't fill up the entire terminal window.

Suggested change
<Text>{output}</Text>
<Box marginTop={1}>
<Text>{output}</Text>
</Box>

}, [setOutput]);

return (
<Box flexDirection="column">
Copy link
Owner

Choose a reason for hiding this comment

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

To give the whole UI more space.

Suggested change
<Box flexDirection="column">
<Box flexDirection="column" padding={1}>

@vadimdemedes
Copy link
Owner

Thanks!

@vadimdemedes vadimdemedes changed the title Add example for writing streams Add example for rendering subprocess output Dec 19, 2020
@vadimdemedes vadimdemedes merged commit 5eb17f3 into vadimdemedes:master Dec 19, 2020
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