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.

Sign up for GitHub

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

Jump to bottom

Fix CLI to avoid waiting for stdin without any input and avoid different outputs #7131

Merged
merged 3 commits into from Aug 14, 2023
Merged

Fix CLI to avoid waiting for stdin without any input and avoid different outputs #7131

merged 3 commits into from Aug 14, 2023

Conversation

ybiquitous
Copy link
Member

Which issue, if any, is this issue related to?

Closes #3401

Is there anything in the PR that needs further explanation?

This commit changes the following:

  1. To avoid waiting for stdin without any input. For example, just running the stylelint command
    immediately exits, instead of waiting. I believe this behavior is more user-friendly.
  2. To avoid different outputs on empty files and empty stdin. This is achieved by allowing an empty
    string input ('') from stdin. See the example below: 
    # code is undefined
bin/stylelint.mjs

# code is ''
echo -n '' | bin/stylelint.mjs

See also process.stdin.isTTY.

@changeset-bot

This comment was marked as resolved.

Sign in to view
@ybiquitous ybiquitous linked an issue Aug 14, 2023 that may be closed by this pull request
Fix different outputs on empty files and empty stdin in CLI #3401
Closed
ybiquitous
ybiquitous commented Aug 14, 2023
View reviewed changes
lib/__tests__/cli.test.mjs
@@ -183,24 +183,22 @@ describe('buildCLI', () => {
});

describe('CLI', () => {
beforeAll(() => {
beforeEach(() => {
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] To avoid effects on each it.

lib/__tests__/cli.test.mjs
expect(process.stdout.write).toHaveBeenCalledTimes(0);
expect(process.stderr.write).toHaveBeenCalledTimes(0);
expect(process.stdout.write).not.toHaveBeenCalled();
expect(process.stderr.write).not.toHaveBeenCalled();
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] To clarify the difference from toHaveBeenCalledWith(0).

@ybiquitous ybiquitous marked this pull request as ready for review August 14, 2023 01:18
@ybiquitous ybiquitous marked this pull request as draft August 14, 2023 02:49
@ybiquitous
Change CLI to avoid waiting for stdin without any input
329c6e1 
This commit changes the following:

1. To avoid waiting for stdin without any input. For example, just running the `stylelint` command
   immediately exits, instead of waiting. I believe this behavior is more user-friendly.
2. To avoid different outputs on empty files and empty stdin. This is achieved by allowing an empty
   string input (`''`) from stdin. See the example below:
   ```sh
   # code is undefined
   bin/stylelint.mjs

   # code is ''
   echo -n '' | bin/stylelint.mjs
   ```

See also [`process.stdin.isTTY`](https://nodejs.org/api/tty.html#readstreamistty).
@ybiquitous ybiquitous marked this pull request as ready for review August 14, 2023 02:50
jeddy3
jeddy3 reviewed Aug 14, 2023
View reviewed changes
Copy link
Member

@jeddy3 jeddy3 left a comment •
edited

Choose a reason for hiding this comment

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

Great to see consistent behaviour for empty files and std.

Changed: CLI to avoid waiting for stdin without any input

We used to show help on npx stylelint. Did we intentionally change it to wait for stdin? If not, then it's a regression, and we can change the entry to patch/fixed and drop the migration guide entry.

@ybiquitous
Copy link
Member Author

We used to show help on npx stylelint. Did we intentionally change it to wait for stdin?

I found out the reason. This bug was mixed in when I removed the get-stdin dependency with PR #6182. 😓

I'll update the changelog and migration guide. Thanks for pointing it out!

@ybiquitous
Copy link
Member Author

FYI. This regression happened with stylelint@14.10.0. See below:

$ npm i -s 'stylelint@14.10.0' && npx stylelint
# waiting for stdin...
$ npm i -s 'stylelint@14.9.1' && npx stylelint | grep 'Usage:'
  Usage: stylelint [input] [options]

@ybiquitous
Copy link
Member Author

I'll update the changelog and migration guide. Thanks for pointing it out!

Fixed via 88bb9b4.

@ybiquitous ybiquitous requested a review from jeddy3 August 14, 2023 07:02
jeddy3
jeddy3 approved these changes Aug 14, 2023
View reviewed changes
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

One minor suggestion for the changelog entry. Otherwise, LGTM thank you!

.changeset/orange-dancers-compete.md Outdated Show resolved Hide resolved
@ybiquitous @jeddy3
Update .changeset/orange-dancers-compete.md
fd70bd1 
Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
@ybiquitous
Copy link
Member Author

Thanks for the review! I've addressed the review on the changelog.
I'll merge this when CI passes. 👍🏼

@ybiquitous ybiquitous changed the title Change CLI to avoid waiting for stdin without any input Fix CLI to avoid waiting for stdin without any input and avoid different outputs Aug 14, 2023
@ybiquitous ybiquitous merged commit 8635c3f into v16 Aug 14, 2023
12 of 14 checks passed
@ybiquitous ybiquitous deleted the change-cli-without-any-input branch August 14, 2023 07:27
@ybiquitous ybiquitous mentioned this pull request Aug 14, 2023
Closed
Mouvedia
Mouvedia reviewed Aug 14, 2023
View reviewed changes
lib/cli.mjs Show resolved Hide resolved
@ybiquitous ybiquitous mentioned this pull request Aug 16, 2023
Closed
ybiquitous added a commit that referenced this pull request Aug 27, 2023
@ybiquitous @jeddy3
Fix CLI to avoid waiting for stdin without any input and avoid differ…
774dcb8 
…ent outputs (#7131)

This commit changes the following:

1. To avoid waiting for stdin without any input. For example, just running the `stylelint` command
   immediately exits, instead of waiting. I believe this behavior is more user-friendly.
2. To avoid different outputs on empty files and empty stdin. This is achieved by allowing an empty
   string input (`''`) from stdin. See the example below:
   ```sh
   # code is undefined
   bin/stylelint.mjs

   # code is ''
   echo -n '' | bin/stylelint.mjs
   ```

Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
ybiquitous added a commit that referenced this pull request Aug 27, 2023
@ybiquitous @jeddy3
Fix CLI to avoid waiting for stdin without any input and avoid differ…
318b6d0 
…ent outputs (#7131)

This commit changes the following:

1. To avoid waiting for stdin without any input. For example, just running the `stylelint` command
   immediately exits, instead of waiting. I believe this behavior is more user-friendly.
2. To avoid different outputs on empty files and empty stdin. This is achieved by allowing an empty
   string input (`''`) from stdin. See the example below:
   ```sh
   # code is undefined
   bin/stylelint.mjs

   # code is ''
   echo -n '' | bin/stylelint.mjs
   ```

Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
ybiquitous added a commit that referenced this pull request Sep 28, 2023
@ybiquitous @jeddy3
Fix CLI to avoid waiting for stdin without any input and avoid differ…
5f8cd9e 
…ent outputs (#7131)

This commit changes the following:

1. To avoid waiting for stdin without any input. For example, just running the `stylelint` command
   immediately exits, instead of waiting. I believe this behavior is more user-friendly.
2. To avoid different outputs on empty files and empty stdin. This is achieved by allowing an empty
   string input (`''`) from stdin. See the example below:
   ```sh
   # code is undefined
   bin/stylelint.mjs

   # code is ''
   echo -n '' | bin/stylelint.mjs
   ```

Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
ybiquitous added a commit that referenced this pull request Oct 19, 2023
@ybiquitous @jeddy3
Fix CLI to avoid waiting for stdin without any input and avoid differ…
000fb3d 
…ent outputs (#7131)

This commit changes the following:

1. To avoid waiting for stdin without any input. For example, just running the `stylelint` command
   immediately exits, instead of waiting. I believe this behavior is more user-friendly.
2. To avoid different outputs on empty files and empty stdin. This is achieved by allowing an empty
   string input (`''`) from stdin. See the example below:
   ```sh
   # code is undefined
   bin/stylelint.mjs

   # code is ''
   echo -n '' | bin/stylelint.mjs
   ```

Co-authored-by: Richard Hallows <jeddy3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mouvedia Mouvedia Mouvedia left review comments

@jeddy3 jeddy3 jeddy3 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix different outputs on empty files and empty stdin in CLI
3 participants
@ybiquitous @Mouvedia @jeddy3