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

add wchar api implementation #10739

Merged
merged 12 commits into from Sep 22, 2023
Merged

add wchar api implementation #10739

merged 12 commits into from Sep 22, 2023

Conversation

extinguish
Copy link

Summary

add wchar api: fputws, putwc, putwchar, fputwc, wcswidth, wcwidth, wcswcs, wcstok, wcsstr, wcsspn, wcspbrk implementation.

Impact

Testing

guoshichao added 11 commits September 21, 2023 15:23
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Signed-off-by: guoshichao <guoshichao@xiaomi.com>
****************************************************************************/

static const unsigned char g_table[] =
{
Copy link
Contributor

Choose a reason for hiding this comment

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

how do you plan to maintain these tables?

Copy link
Author

Choose a reason for hiding this comment

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

should I place the content g_table[] to a separate header file ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The code base comes from musl:
https://github.com/bminor/musl/blob/master/src/ctype/wide.h
https://github.com/bminor/musl/blob/master/src/ctype/nonspacing.h
https://github.com/bminor/musl/blob/master/src/ctype/wcwidth.c#L3-L9
@extinguish just reformat the code to follow NuttX coding style.
so, it may better to ask musl owner to improve the design and comment, so we can incorporate the improvement back.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's our problem to reformat imported code and make maintenance harder.
it isn't a problem in musl.

Copy link
Contributor

Choose a reason for hiding this comment

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

the original code also lack any information about these two tables. Why do you consider it is the reformat problem?

Copy link
Contributor

Choose a reason for hiding this comment

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

at least it's easier to copy changes from the upstream if we don't reformat.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but the code incorporated into nuttx has to conform the coding style which we must follow.
BTW, this is the rule enforced by ci, the rule relaxation needs to be discussed in dev@ list.

Signed-off-by: guoshichao <guoshichao@xiaomi.com>
@xiaoxiang781216 xiaoxiang781216 merged commit abfb7da into apache:master Sep 22, 2023
26 checks passed
@jerpelea jerpelea added this to To-Add in Release Notes - 12.3.0 Sep 26, 2023
@jerpelea jerpelea moved this from To-Add to done in Release Notes - 12.3.0 Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants