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

feat: Implement newline normalization. #228

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

alilleybrinker
Copy link
Member

This should ensure consistent Artifact IDs whether on Windows or Unix systems.

@alilleybrinker alilleybrinker force-pushed the alilleybrinker/newline-normalization branch 2 times, most recently from 74d53cd to 7fe45d7 Compare November 19, 2024 00:02
@alilleybrinker
Copy link
Member Author

Looks like my implementation isn't quite right yet. I don't have a Windows machine handy, so I'm just relying on CI here, which isn't ideal. Nonetheless, I'll get it sorted out.

@alilleybrinker alilleybrinker force-pushed the alilleybrinker/newline-normalization branch 2 times, most recently from 11bd89b to a21bc33 Compare November 19, 2024 04:39
@alilleybrinker alilleybrinker force-pushed the alilleybrinker/newline-normalization branch from f6927fa to de1c5c9 Compare January 11, 2025 00:10
@alilleybrinker
Copy link
Member Author

Haha, the implementation is working but some lints are failing. I'll have to come back to fix that later.

@alilleybrinker alilleybrinker force-pushed the alilleybrinker/newline-normalization branch from de1c5c9 to 5223a27 Compare January 14, 2025 18:56
@alilleybrinker
Copy link
Member Author

Looks like newline normalization is still not quite right; errors on Windows.

@alilleybrinker alilleybrinker force-pushed the alilleybrinker/newline-normalization branch from 337a77b to 6dcbdbb Compare January 29, 2025 21:10
This should ensure consistent Artifact IDs whether on Windows or
Unix systems. It works by normalizing all CRLF newlines into LF
newlines unconditionally. Since we don't care about roundtripping
data (we don't store data at all, unlike Git), we can do this
and in fact *have* to do it unconditionally to make the ID
system work.

Signed-off-by: Andrew Lilley Brinker <[email protected]>
@alilleybrinker alilleybrinker force-pushed the alilleybrinker/newline-normalization branch from 6dcbdbb to a8cda73 Compare January 29, 2025 21:12
@alilleybrinker
Copy link
Member Author

Haha! Finally!! Got it working on Windows.

This also ended up involving some splitting out of code from the gitoid module in the gitoid crate into separate modules, which should hopefully make it easier / clearer to maintain over time.

@alilleybrinker alilleybrinker merged commit 083f0f4 into main Jan 29, 2025
10 checks passed
@alilleybrinker alilleybrinker deleted the alilleybrinker/newline-normalization branch January 29, 2025 21:22
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.

1 participant