Skip to content

Conversation

anlutro
Copy link
Contributor

@anlutro anlutro commented Sep 18, 2014

Had to move const HINT_PATH_DELIMITER to the ViewFinderInterface. static::HINT_PATH_DELIMITER in classes implementing the interface still works. http://3v4l.org/Z616Z

This does prevent end users from defining their own namespace delimiter. We could make a method ViewFinderInterface->getDelimiter() method instead. Thoughts?

Ref #5756 (comment)

Had to move HINT_PATH_DELIMITER to the ViewFinderInterface.
static::HINT_PATH_DELIMITER still works. http://3v4l.org/Z616Z
@crynobone
Copy link
Member

We could make a method ViewFinderInterface->getDelimiter() method instead. Thoughts?

I don't see much advantage on allowing it to be configurable. From a package developer perspective, we already following the default :: convention, and if app developer modify this, it would still break package code.

@anlutro
Copy link
Contributor Author

anlutro commented Sep 18, 2014

Good point, I agree.

@anlutro anlutro changed the title [5.0] [WIP] Normalize view names with namespaces with slashes properly [5.0] Normalize view names with namespaces with slashes properly Sep 18, 2014
@jasonlewis
Copy link
Contributor

How often is it used? Even worth having it as a constant?

@anlutro
Copy link
Contributor Author

anlutro commented Sep 18, 2014

Well, both the View\Factory and FileViewFinder need to know about it, so a constant does make sense as it's easily shared between the two.

@jasonlewis
Copy link
Contributor

I figured since it can't be changed why not hard code it in. What's the benefits of keeping it as a constant? Only thing I can think of is if someone were to implement their own finder and they want to use the constant in case it were to ever change, for whatever reason.


list($namespace, $name) = explode($delimiter, $name);

return $namespace . $delimiter . str_replace('/', '.', $name);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll beat our man @GrahamCampbell to it. No need for the spaces when concatenating. 👼

@anlutro
Copy link
Contributor Author

anlutro commented Sep 18, 2014

Constants are great, man. Even if in theory '::' for view namespace delimiters will never be changed to something else, having it defined in one place and one place only eases my mind - I don't like writing code that requires a search/replace to change something later on.

@GrahamCampbell
Copy link
Member

👍

taylorotwell added a commit that referenced this pull request Sep 18, 2014
[5.0] Normalize view names with namespaces with slashes properly
@taylorotwell taylorotwell merged commit beb5b95 into laravel:master Sep 18, 2014
@patrickheeney
Copy link

This is nice. Could this be done to anything that uses namespaces: configs and lang?

@anlutro
Copy link
Contributor Author

anlutro commented Sep 18, 2014

I don't think so, because in config/lang files there can be multidimensional arrays. For example... Config::get('one/two.three.four') will open the file config/one/two.php and look for the array key ['three']['four']. If you replace the / with a . or vice-versa you get completely different behaviour.

@anlutro
Copy link
Contributor Author

anlutro commented Sep 18, 2014

This is also why I'd prefer slashes over dots - it's more consistent with config/lang file behaviour.

@patrickheeney
Copy link

EDIT: Disregard, I see you don't normalize the namespace with dots.

This would be for the namespace for consistency. Config::get('project.namespace::one/two.three.four'). I agree with the later part though, it would be difficult for folder behavior with the dot notation. With this change merged, I would suspect the namespaces to work the same everywhere.

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.

6 participants