Skip to content

fix(plugin-commands-patching): leave final newline in patch files#6905

Merged
zkochan merged 2 commits intopnpm:mainfrom
andersk:patch-newline
Aug 17, 2023
Merged

fix(plugin-commands-patching): leave final newline in patch files#6905
zkochan merged 2 commits intopnpm:mainfrom
andersk:patch-newline

Conversation

@andersk
Copy link
Copy Markdown
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.

@andersk
fix(plugin-commands-patching): leave final newline in patch files
326ac7e 
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
Copy Markdown

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
Copy Markdown
Member

zkochan commented Aug 3, 2023

What issue is the removed newline is causing?

@zkochan
Copy link
Copy Markdown
Member

zkochan commented Aug 16, 2023

@andersk could you answer?

@andersk
Copy link
Copy Markdown
Contributor Author

andersk commented Aug 17, 2023 •
edited
Loading

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
@welcome
Copy link
Copy Markdown

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

Reviewers

@zkochan zkochan zkochan approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@andersk @zkochan