Skip to content

Added macros print! and println! #430

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Aug 1, 2022
Merged

Added macros print! and println! #430

merged 12 commits into from
Aug 1, 2022

Conversation

JonahPlusPlus
Copy link
Contributor

Added two macros to print to stdout. If SYSTEM_TABLE is None, they panic.

@JonahPlusPlus
Copy link
Contributor Author

Note: I used core::format_args! inside of itself in println!, instead of core::format_args_nl! since that requires users to add a feature.

JonahPlusPlus
JonahPlusPlus commented May 8, 2022
View reviewed changes
GabrielMajeri
GabrielMajeri requested changes May 16, 2022
View reviewed changes
Copy link
Collaborator

@GabrielMajeri GabrielMajeri left a comment

Choose a reason for hiding this comment

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

Code looks good overall! I've left some small feedback comments, and also noticed that CI failed due to formatting issues

GabrielMajeri
GabrielMajeri reviewed May 19, 2022
View reviewed changes
Copy link
Collaborator

@GabrielMajeri GabrielMajeri left a comment

Choose a reason for hiding this comment

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

Code looks good overall! I've just realized we don't have any tests for these new macros. Could you call them somewhere in the uefi-test-runner crate? We could replace some info! call with println!, for example.

@JonahPlusPlus
Copy link
Contributor Author

OK, will look into it.

@GabrielMajeri
Copy link
Collaborator

Hi @JonahPlusPlus - do you have a status update on this PR?

@JonahPlusPlus
Added print! and println! tests
38f591d 
Tests formatting with macros.
@JonahPlusPlus
Update main.rs
0386857 
Forgot period
@JonahPlusPlus
Copy link
Contributor Author

Hi @JonahPlusPlus - do you have a status update on this PR?

Apologies, this slipped my mind with other projects.

I added print! and println! calls with formatting to demonstrate their use.

@JonahPlusPlus
Update main.rs
5eeb2dc 
Formatting
@JonahPlusPlus
Update main.rs
531f28c 
clippy doesn't like constant strs in formatting
@JonahPlusPlus
Update main.rs
2dd3f35 
clippy again
GabrielMajeri
GabrielMajeri approved these changes Aug 1, 2022
View reviewed changes
Copy link
Collaborator

@GabrielMajeri GabrielMajeri left a comment

Choose a reason for hiding this comment

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

Thank you, the PR looks good now!

@GabrielMajeri GabrielMajeri changed the title Added macros print! and println! Added macros print! and println! Aug 1, 2022
@GabrielMajeri GabrielMajeri merged commit 77dc2fd into rust-osdev:main Aug 1, 2022
@JonahPlusPlus JonahPlusPlus deleted the print branch November 18, 2024 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GabrielMajeri GabrielMajeri GabrielMajeri 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
@JonahPlusPlus @GabrielMajeri