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

fix: ensure command output ends with newline #3565

Merged
merged 3 commits into from
Jul 10, 2021

Conversation

lukashass
Copy link
Contributor

I found some missing newlines at the end of the output of pnpm audit and pnpm list.
When looking at plugin-command-listing I found that adding newlines there might be cumbersome, so I added it just before writing to stdout.

I also added some types, because the template literal I added complained about any.

@lukashass lukashass requested a review from zkochan as a code owner June 26, 2021 10:29
@zkochan
Copy link
Member

zkochan commented Jul 2, 2021

I don't know if it is a good idea. In some terminals it might look bad, adding an extra empty line.

@owl-from-hogvarts
Copy link

I don't know if it is a good idea. In some terminals it might look bad, adding an extra empty line.

But in other ones it just breaks them)
image

IMO thats better put up with "bad look" to not to break things
We can enshure that only one new line is on the end of output.

@owl-from-hogvarts
Copy link

owl-from-hogvarts commented Jul 7, 2021

I can't approve this PR since it proposes crutch: you are not solving the issue where it occurs, but performs check somewhere after the problems occure and then doing something to return result to expected behavior, correcting it (in this case adding newline char). It is exact as crutch looks, because you fix it in that particular example i.e. for that particular case. But what about other cases? What if thiss issue can occure some where else?

IMO it would be much better to determine real place where problem occure. I guess it is logger, or call to logger... in general, something related to the logger.

@owl-from-hogvarts
Copy link

My fault. Not a logger but reporter. Here, logger is responsible for i guess pnpm-debug.log but not for communicating with user through terminal

@zkochan zkochan merged commit a1b85a7 into pnpm:main Jul 10, 2021
@zkochan zkochan added this to the v6.10 milestone Jul 11, 2021
@lukashass lukashass deleted the ensure-ouput-ends-with-newline branch August 15, 2021 21:18
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