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

Improve documentation of the usage section #541

Merged
merged 3 commits into from Mar 9, 2023
Merged

Improve documentation of the usage section #541

merged 3 commits into from Mar 9, 2023

Conversation

ehmicky
Copy link
Collaborator

@ehmicky ehmicky commented Mar 7, 2023

This improves the Usage section of the documentation. That section has been augmented ad-hoc incrementally over time and could use some small improvement. It also removes some information that is not strictly worth documenting, as it helps highlighting the features that are more interesting.

  • Some of the scripting interface examples are slightly redundant, so I combined them into fewer examples, while keeping the same information.
  • Move the signal option's example to the Tips sections, since it is not really a feature anymore now that the option is supported by the child_process core module.
  • Remove examples of the synchronous methods, as the async version should usually (although not always) be preferred. The synchronous methods are documented in the API, and are pretty straightforward once you know the related async method.
  • Small description changes

I am hoping those changes help the Usage section to better focus and highlight the major features that Execa does bring.

readme.md Outdated
Comment on lines 56 to 63
const unicorns = await $`echo unicorns`;
const {stdout} = await $`echo ${unicorns}${'!'}}`;
console.log(stdout);
//=> 'unicorns!'

const {stdout} = await $`echo ${['unicorns', 'rainbows']}`;
console.log(stdout);
//=> 'unicorns'
//=> 'unicorns rainbows'
Copy link
Contributor

Choose a reason for hiding this comment

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

This example errors on reassignment of the block-scoped var stdout

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right! Fixed.

readme.md Outdated

const {stdout} = $.sync`echo unicorns`;
console.log(stdout);
await $$({shell: true})`echo unicorns && echo rainbows`;
Copy link
Contributor

Choose a reason for hiding this comment

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

This consolidation is nice, but removes the basic example of using the imported $ directly with options e.g. await $(options)`command`. I'm not sure if this will lead to confusion, but user might think they have to use the $$ = $(options) binding mechanism first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point, let's keep them split. Done 👍

@sindresorhus sindresorhus changed the title Improve Documentation of the Usage section Improve documentation of the usage section Mar 9, 2023
@ehmicky
Copy link
Collaborator Author

ehmicky commented Mar 9, 2023

Fixed @aaronccasanova, ready for another code review 👍

@ehmicky ehmicky merged commit 5320fef into main Mar 9, 2023
20 checks passed
@ehmicky ehmicky deleted the doc-usage branch March 9, 2023 02:41
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.

None yet

3 participants