Skip to content

Conversation

@spaceone
Copy link
Contributor

@spaceone spaceone commented Sep 27, 2025

encode DEL control characters in values, and remove them from keys.
Fixes: 3f5ba89

encode DEL control characters in values, and remove them from keys.

Fixes: 3f5ba89
@spaceone
Copy link
Contributor Author

Now, the DEL control char is also removed from a key, and a test case for both variants has been added.

@ChrisHines ChrisHines changed the title fix: encode DEL (0x7f) control character (#17) fix: encode DEL (0x7f) control character Oct 3, 2025
@ChrisHines
Copy link
Member

I don't understand what GitHub is doing. I was trying to pull in your latest changes to review them and somehow that resulted in merging two branches that were from different force pushes you did. But the most surprising thing is that somehow that managed to also put a merge commit into your fork! I've never seen that before.

I think the safest thing here might be to close this PR and for you to submit a new PR with your latest version of the code so I can review the right code because whatever just happened broke your changes.

Sorry about that.

@spaceone
Copy link
Contributor Author

spaceone commented Oct 3, 2025

hm, let me check If I can fix it.

@spaceone spaceone force-pushed the fix/escape-del-char branch from 7d9ecee to 0990210 Compare October 3, 2025 00:43
@spaceone
Copy link
Contributor Author

spaceone commented Oct 3, 2025

@ChrisHines Forced pushed the correct state again. All tests are green. You should just be able to merge?

Copy link
Member

@ChrisHines ChrisHines left a comment

Choose a reason for hiding this comment

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

Ah, thanks. That cleaned up the weirdness.

Copy link
Member

@ChrisHines ChrisHines left a comment

Choose a reason for hiding this comment

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

Thanks for the fix.

@ChrisHines ChrisHines merged commit d0d028a into go-logfmt:main Oct 3, 2025
21 checks passed
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