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(plugin-commands-patching): leave final newline in patch files #6905

Merged
merged 2 commits into from Aug 17, 2023

Conversation

andersk
Copy link
Contributor

@andersk andersk commented Aug 3, 2023

The safe-execa library strips the final newline from the command output by default. Here it should be left in.

The safe-execa library strips the final newline from the command
output by default.  Here it should be left in.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk andersk requested a review from zkochan as a code owner August 3, 2023 21:23
@welcome
Copy link

welcome bot commented Aug 3, 2023

💖 Thanks for opening this pull request! 💖
Please be patient and we will get back to you as soon as we can.

@zkochan
Copy link
Member

zkochan commented Aug 3, 2023

What issue is the removed newline is causing?

@zkochan
Copy link
Member

zkochan commented Aug 16, 2023

@andersk could you answer?

@andersk
Copy link
Contributor Author

andersk commented Aug 17, 2023

Patch files, like all text files, should always end with a trailing newline. Patch files generated by diff and git diff end with a trailing newline, so ours should too. If the newline is missing, GitHub displays a red warning icon (example), git diff and git show warn \ No newline at end of file, and cat will incorrectly combine multiple files or cause your shell prompt to unexpectedly start in the middle of a line. This might be unlikely to cause “real” problems in practice, but many programmers like being tidy with their source trees, so their tools should be tidy with it too.

@zkochan zkochan merged commit 512d712 into pnpm:main Aug 17, 2023
7 checks passed
@welcome
Copy link

welcome bot commented Aug 17, 2023

Congrats on merging your first pull request! 🎉🎉🎉

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

2 participants