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

[WIP] Add ResourceRetriever::getFilePath() #970

Closed
wants to merge 2 commits into from

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Feb 4, 2018

Motivation
In some cases, it is required to extract the (absolute) file path of a URI (if available). For example, MeshShape needs to know the path to a mesh file to infer the path to a texture image, which is given as a relative path. Currently, we don't have a public method to get a file path from a URI unless it's a file URI. PackageResourceRetriever and DartResourceRetriever have similar functionality, but it's not public.

Proposal
I propose adding ResourceRetriever::getFilePath() that returns the absolute file path specified by a URI or an empty string if unavailable.


Before creating a pull request

  • Document new methods and classes

Before merging a pull request

  • Set version target by selecting a milestone on the right side
  • Summarize this change in CHANGELOG.md
  • Add unit test(s) for this change

@jslee02 jslee02 added this to the DART 6.4.0 milestone Feb 4, 2018
@jslee02 jslee02 requested review from mkoval and mxgrey February 4, 2018 21:29
@jslee02 jslee02 changed the base branch from master to release-6.4 February 4, 2018 21:48
@jslee02 jslee02 changed the base branch from release-6.4 to master February 4, 2018 21:48
Copy link
Member

@mxgrey mxgrey left a comment

Choose a reason for hiding this comment

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

We could consider adding a small unit test to ensure coverage of the new function, but it's likely already covered through the exists(~) function, so maybe it isn't important.

@jslee02
Copy link
Member Author

jslee02 commented Feb 4, 2018

Sure, I wanted to hear from you (and possibly @mkoval) if this sounds before adding tests. 😄

Btw, I meant to add this to DART 6.4 (and later by cascading the changes) but unintentionally made this branching from master. Let me create a new PR for DART 6.4. 😓

@jslee02 jslee02 mentioned this pull request Feb 5, 2018
4 tasks
@jslee02 jslee02 closed this Feb 5, 2018
@jslee02 jslee02 deleted the 6.4/resource_retriever_getfilepath branch February 12, 2018 03:26
@jslee02 jslee02 removed this from the DART 6.4.0 milestone Feb 14, 2018
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.

2 participants