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

add missing eof line support #11

Merged
merged 3 commits into from
Feb 28, 2023
Merged

add missing eof line support #11

merged 3 commits into from
Feb 28, 2023

Conversation

zmully
Copy link
Contributor

@zmully zmully commented Feb 25, 2023

Adds support for missing eof marker in git diffs. Without this, the parser will only parse the diff up to the first instance of this marker in the diff.

@yeonjuan I don't actually have any use for this in the parsed output, but discovered this issue when I was experimenting with your library. I'm also not sure if this is the only git diff line that uses the \ character, the git docs are rather sparse on this matter.

@yeonjuan
Copy link
Owner

yeonjuan commented Feb 26, 2023

HI, @zmully Thanks for fixing it!

I don't actually have any use for this in the parsed output, but discovered this issue when I was experimenting with your library. I'm also not sure if this is the only git diff line that uses the \ character, the git docs are rather sparse on this matter.

After a quick check of the git source, it seems that the "" character is always followed by a "No newline at end of file" message. But I thinks it would be great if we use general type value such as "Message" for the handling other messages we might miss.

{
   type: "Message",
   content: "No newline at end of file"
}

@yeonjuan yeonjuan self-requested a review February 26, 2023 05:53
@zmully
Copy link
Contributor Author

zmully commented Feb 26, 2023

Thanks @yeonjuan for the quick review and feedback! I've updated the PR with your feedback and added the missing README section I didn't catch the first time around. Thanks for putting together this library, it is super useful, and helped me put together an internal tool to parse out stringified JSON (like usually seen in IAM policy declarations) in diffs and redisplay the changes in a format that makes reviewing changes easier and less error prone.

Copy link
Owner

@yeonjuan yeonjuan left a comment

Choose a reason for hiding this comment

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

LGTM!, Thanks for contribution!

@yeonjuan yeonjuan merged commit b96afe9 into yeonjuan:main Feb 28, 2023
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.

2 participants