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

Include BUILD files in Xcode's Project navigator #301

Conversation

jpsim
Copy link
Contributor

@jpsim jpsim commented Apr 15, 2022

Resolves #300

Before After
image image

transitive.append(inputs.srcs)
transitive.append(inputs.non_arc_srcs)
transitive.append(inputs.hdrs)
new_srcs = []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is super verbose, I'm sure there's a nicer way to do this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I guess this is why I just had them as File.

I wonder if it would be better to have a second extra_file_paths instead of changing the type of extra_files? Then in xcodeproj.bzl we append extra_file_paths to the transformed extra_files and call to_dto on that?

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 don't feel strongly either way. I have a slight preference for keeping it as-is because I think this mapping is still simpler than having two ways to add extra files.

I was more asking if there's a more concise way to perform this mapping of a depset. Like inputs.srcs.map(file_path).

Copy link
Contributor

@brentleyjones brentleyjones Apr 15, 2022

Choose a reason for hiding this comment

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

The downside to this way is that we are cracking open the depset for each target, and if there are files that are duplicated in the inputs it's more efficient to only do it once later.

I'm fine with it like this though, since I'll be refactoring stuff later. Better to keep it localized for now.

As for your question:

transitive.extend([
    file_path(file)
    for file in inputs.srcs.to_list()
])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I didn't notice this syntax elsewhere, it's surprising to me because it's wrapped in square brackets already. e53f82f

@jpsim jpsim force-pushed the include-build-files-in-xcodes-project-navigator branch from 6646444 to c28e482 Compare April 15, 2022 16:46
@jpsim jpsim marked this pull request as ready for review April 15, 2022 16:46
@jpsim jpsim changed the title Include BUILD files in Xcode's Project navigator Include BUILD files in Xcode's Project navigator Apr 15, 2022
@brentleyjones brentleyjones force-pushed the include-build-files-in-xcodes-project-navigator branch from e53f82f to 8cba5c0 Compare April 15, 2022 17:41
@brentleyjones
Copy link
Contributor

(Trying to debug a CI issue, sorry for the noise.)

@brentleyjones brentleyjones merged commit c5f70a4 into MobileNativeFoundation:main Apr 15, 2022
@jpsim jpsim deleted the include-build-files-in-xcodes-project-navigator branch April 15, 2022 18:06
@jpsim
Copy link
Contributor Author

jpsim commented Apr 15, 2022

Thanks for guiding me through this @brentleyjones! 🙌

@brentleyjones
Copy link
Contributor

Thanks for the contribution!

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.

Feature Request: Show BUILD files in the Project navigator
3 participants