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.

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

ICU-22049 fix layoutex #2106

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

ICU-22049 fix layoutex #2106

wants to merge 1 commit into from

Conversation

srl295
Copy link
Member

@srl295 srl295 commented May 31, 2022 •
edited

  • detect 'another' icu4c via pkgconfig and use it
  • copy some internal files into layoutex to make it usable
Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22049
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@srl295
ICU-22049 fix layoutex
3b5a1d9 
- detect 'another' icu4c via pkgconfig and use it
- copy some internal files into layoutex to make it usable
@srl295 srl295 added the incomplete Needs work; do not approve/merge as is. label May 31, 2022
@srl295 srl295 self-assigned this May 31, 2022
srl295
srl295 commented May 31, 2022
View reviewed changes
icu4c/source/layoutex/cmemory.h
@@ -0,0 +1 @@
../common/cmemory.h
Copy link
Member Author

Choose a reason for hiding this comment

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

symlink. may not work everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

Do these symlinks replace some kind of manual step of copying files?
Shouldn't these be handled by setting a -I include folder path?

@srl295
Copy link
Member Author

srl295 commented Nov 10, 2022

I'm not sure what work this still needs. Maybe docs. Per June 7 2022 it was working for Quark folks.

markusicu
markusicu reviewed Nov 12, 2022
View reviewed changes
icu4c/source/layoutex/ParagraphLayout.cpp
Comment on lines +33 to +40
#ifndef FALSE
#define FALSE false
#endif

#ifndef TRUE
#define TRUE true
#endif

Copy link
Member

Choose a reason for hiding this comment

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

no longer needed after #2175

Copy link
Member

Choose a reason for hiding this comment

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

ping

icu4c/source/layoutex/RunArrays.cpp
Comment on lines +19 to +27
#ifndef FALSE
#define FALSE false
#endif

#ifndef TRUE
#define TRUE true
#endif


Copy link
Member

Choose a reason for hiding this comment

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

no longer needed after #2175

Copy link
Member

Choose a reason for hiding this comment

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

ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markusicu markusicu markusicu left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@srl295 srl295

Labels
incomplete Needs work; do not approve/merge as is.
Projects
None yet
Milestone
No milestone
2 participants
@srl295 @markusicu