Skip to content

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

Merged
merged 1 commit into from
Mar 13, 2019

Conversation

galpeter
Copy link
Contributor

@galpeter galpeter commented Mar 5, 2019

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 for the
jerry-default-port for the jerry-ext headers.

void
jerry_port_print_char (char c) /**< the character to print */
{
printf ("%c", c);
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Member

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.

Copy link
Contributor

@LaszloLango LaszloLango left a 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
@galpeter
Copy link
Contributor Author

@dbatyai I've updated the PR. Now the putchar is used in the default port

Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

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

LGTM (informal)

@dbatyai
Copy link
Member

dbatyai commented Mar 13, 2019

lgtm

@dbatyai dbatyai merged commit 162ba8c into jerryscript-project:master Mar 13, 2019
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.

4 participants