-
Notifications
You must be signed in to change notification settings - Fork 687
Make jerryx_port_handler_print_char truly a port function #2789
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
Conversation
jerry-port/default/default-io.c
Outdated
| void | ||
| jerry_port_print_char (char c) /**< the character to print */ | ||
| { | ||
| printf ("%c", c); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the purpose of this function is to print a single character, would it make sense to use putchar? Could that make a difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not against it, currently I've just moved the original implementation. AFAIK if there is a printf on a port, the putchar should also be there. So it should not change anything. Should I change it to putchar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both printf and putchar are good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think putchar would be preferable here, but if you feel that it's outside of the scope of this change, then I won't insist.
LaszloLango
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
In the previous approach `jerryx_port_handler_print_char` was implemented in by the jerry-default-port. This implementation however required the jerry-ext handler header file which created a requirement in the jerry-default-port for the jerry-ext headers. JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com
ce260ab to
0925184
Compare
|
@dbatyai I've updated the PR. Now the putchar is used in the default port |
rerobika
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (informal)
|
lgtm |
In the previous approach
jerryx_port_handler_print_charwas implementedin by the jerry-default-port. This implementation however required the
jerry-ext handler header file which created a requirement for the
jerry-default-port for the jerry-ext headers.