Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Add missing includes #6670

Merged
mehmetf merged 1 commit into from
Oct 26, 2018
Merged

Add missing includes #6670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/ui/isolate_name_server/isolate_name_server_natives.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef FLUTTER_LIB_UI_ISOLATE_NAME_SERVER_NATIVES_H_
#define FLUTTER_LIB_UI_ISOLATE_NAME_SERVER_NATIVES_H_

#include <string>
#include "third_party/dart/runtime/include/dart_api.h"

namespace tonic {
Expand Down
1 change: 1 addition & 0 deletions lib/ui/text/asset_manager_font_provider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "flutter/assets/asset_manager.h"
#include "flutter/fml/macros.h"
#include "third_party/skia/include/core/SkFontMgr.h"
#include "third_party/skia/include/core/SkTypeface.h"
#include "txt/font_asset_provider.h"

namespace blink {
Expand Down
3 changes: 3 additions & 0 deletions third_party/txt/src/minikin/GraphemeBreak.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
#ifndef MINIKIN_GRAPHEME_BREAK_H
#define MINIKIN_GRAPHEME_BREAK_H

#include <stddef.h>
Copy link
Contributor

@jamesderlin jamesderlin Oct 26, 2018

Choose a reason for hiding this comment

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

Since this is C++, #include <cstddef> would be more appropriate.

But since this is in third_party, is this the appropriate place to make this change? Should it not be further upstreamed elsewhere?

Copy link
Contributor Author

@mehmetf mehmetf Oct 26, 2018

Choose a reason for hiding this comment

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

I used <stddef.h> because other files use it too. Let's be consistent. Migration can be done but that's not the point of this PR.

I agree on the third_party directory. It was odd to me at first as well. I believe despite this being under "third_party" it is managed by Flutter team. I see lots of direct PRs on it and we rename it internally to root it under flutter_engine while keeping all the license files intact.

Copy link
Contributor

@bkonyi bkonyi Oct 26, 2018

Choose a reason for hiding this comment

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

third_party/txt is a weird case since there isn't an upstream repository. As far as I know, libtxt is totally contained and maintained in this repository. @jason-simmons would know more.

Copy link
Member

@jason-simmons jason-simmons Oct 26, 2018

Choose a reason for hiding this comment

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

Within third_party/txt, the src/minikin tree is derived from a component of Android. The src/txt tree is specific to Flutter.

Copy link
Contributor Author

@mehmetf mehmetf Oct 26, 2018

Choose a reason for hiding this comment

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

But we still don't roll minikin, right? It is a fork. I see direct commits there as well: 058edef#diff-af8af38ff27b8f206b45f42a2a3f2f2b

Copy link
Member

@jason-simmons jason-simmons Oct 26, 2018

Choose a reason for hiding this comment

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

Currently it's effectively a fork. We might want to do a roll at some point, but we also have local patches that would need to be merged.

#include <unicode/utf16.h>
Copy link
Contributor

@bkonyi bkonyi Oct 26, 2018

Choose a reason for hiding this comment

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

Where is this include needed?

Copy link
Contributor Author

@mehmetf mehmetf Oct 26, 2018

Choose a reason for hiding this comment

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

Using size_t and uint16_t constructs. Not available without these imports. Internally we have checks that make sure each header can be parsed/compiled individually.

If this is still weird, let's chat.


namespace minikin {

class GraphemeBreak {
Expand Down