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 stderr redirection #3479

Merged
merged 1 commit into from
Apr 9, 2024
Merged

Fix stderr redirection #3479

merged 1 commit into from
Apr 9, 2024

Conversation

eldios
Copy link
Contributor

@eldios eldios commented Apr 7, 2024

  • PR Description

Seems that there's a problem in the Stdout/Stderr/Stdin stream vars assignments,
probably copy-paste issue.

If this is intentional, I suggest adding an explanation on why Stderr -> Stdout intentionally to avoid other PRs and confused people around :)

Thanks!

  • Please check if the PR fulfills these requirements
  • Cheatsheets are up-to-date (run go generate ./...)
  • Code has been formatted (see here)
  • Tests have been added/updated (see here for the integration test guide)
  • Text is internationalised (see here)
  • Docs (specifically docs/Config.md) have been updated if necessary
  • You've read through your own file changes for silly mistakes etc

@stefanhaller
Copy link
Collaborator

Looks right to me. The code was introduced in bd2170a, and that diff shows that it was probably unintentional.

But I'm curious if this fixes an actual bug that you observed, or if you were just reading the code and thought it was wrong. Can you think of a situation where this changes user-observable behaviour?

@eldios
Copy link
Contributor Author

eldios commented Apr 8, 2024

found by chance while working on PR#3478

Honestly, not sure about a situation where it would change the perceivable UX/UI.

Repository owner deleted a comment from eldios Apr 8, 2024
Seems that there's a problem in the Stdout/Stderr/Stdin vars
assignments, probably copy-paste issue.
@stefanhaller stefanhaller added the ignore-for-release This will exclude the PR from release notes label Apr 9, 2024
@stefanhaller stefanhaller enabled auto-merge April 9, 2024 07:08
@stefanhaller stefanhaller merged commit 4ba8560 into jesseduffield:master Apr 9, 2024
13 of 14 checks passed
@eldios eldios deleted the typo/fix-stderr-redirection branch April 9, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release This will exclude the PR from release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants