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

Handling subdirectories in document folder #500

Closed
wants to merge 13 commits into from
Closed

Handling subdirectories in document folder #500

wants to merge 13 commits into from

Conversation

sidux
Copy link

@sidux sidux commented Dec 17, 2015

No description provided.

@saimaz
Copy link
Member

saimaz commented Dec 17, 2015

May you provide a test case for this functionality?

@mvar
Copy link
Member

mvar commented Dec 17, 2015

As we follow PSR-4 getFileNamespace() should use path to extract full namespace.

@saimaz
Copy link
Member

saimaz commented Dec 17, 2015

@sidux can you do a rebase with master branch, we made some changes to ensure support for symfony 3.0, otherwise all tests will fail.

@sidux
Copy link
Author

sidux commented Dec 18, 2015

@mvar yeah you are right but how am i supposed to know namespace root from document full path ?

@saimaz
Copy link
Member

saimaz commented Dec 21, 2015

@sidux yoou, this namespace grab doesn't look healthy.. Can't you get namespace from some Reflection ?

@sidux
Copy link
Author

sidux commented Dec 22, 2015

@saimaz you can't use files with Reflection, the only thing i have there is the file path, but i can still read the composer.json and decode it to get the "autoload.psr-4" param so i can get the project root namespace.

@mvar
Copy link
Member

mvar commented Dec 22, 2015

@sidux there is a way to get full namespace without parsing any files.

In getBundleDocumentPaths() you have first part of namespace (e.g., Acme\DemoBundle). And when you [recursively] get the list of documents in the Document/ directory using glob(), you have the last part (plus .php extension). So full namespace would be like this (pseudo code):

BUNDLE_NAMESPACE + '\Document\' + (FILE_PATH_FROM_GLOB - EXTENSION)

You can change result format of getBundleDocumentPaths(). You need to return full namespace of each document instead of paths. You can also rename this method to getBundleDocumentNamespaces(). This will introduce BC break, but that's totally fine. These changes will be released with v1.0 (in early January).

Few notes:

  • Wouldn't it be nicer to use RecursiveDirectoryIterator class instead of glob() function?
  • It looks like you did git merge instead of rebase (your branch duplicates existing commits)

Thanks for contributing!

@mvar
Copy link
Member

mvar commented Jan 4, 2016

@sidux what's the status of this PR?

btw, happy New Year!

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.

None yet

5 participants