Skip to content

Update port.rs to do \r\n #40

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

Merged
merged 2 commits into from
Jul 24, 2025
Merged

Update port.rs to do \r\n #40

merged 2 commits into from
Jul 24, 2025

Conversation

rsahwe
Copy link
Contributor

@rsahwe rsahwe commented Jul 24, 2025

rust uses only \n regardless of platform and some serial monitors such as the qemu one need \r\n. This would fix this.

rust uses only \n regardless of platform and some serial monitors such as the qemu one need \r\n. This would fix this.
@phil-opp
Copy link
Member

Thanks for the PR! Seems reasonable to me.

Could you document in the method docs that send does this replacement and link to send_raw as alternative?

I just noticed that we also do some replacements for 8 | 0x7F bytes. I don't remember why we did this, perhaps you have an idea? Either way, we should document this replacement too.

@rsahwe
Copy link
Contributor Author

rsahwe commented Jul 24, 2025

8 is backspace and the replacement explicitly overwrites the character with space, probably because sometimes backspace only moves the cursor. I don't know why 0x7f (delete) also does backspace though.

@phil-opp
Copy link
Member

Thanks!

I don't know why 0x7f (delete) also does backspace though.

Seems a bit weird to me too. What do you think? Should we remove that replacement?

@rsahwe
Copy link
Contributor Author

rsahwe commented Jul 24, 2025

I don't know, this has apparently always been a part of the crate and this might be some weird uart thing where delete does not exist. Maybe ask the person who wrote that part.

@phil-opp phil-opp merged commit 4a189cd into rust-osdev:master Jul 24, 2025
4 checks passed
phil-opp added a commit that referenced this pull request Jul 24, 2025
@phil-opp
Copy link
Member

Published as v0.4.0.

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