-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Inconsistent syntax for creating links #2426
Comments
I agree, and vote for using array to route and string to relative url |
i agree with this issue |
i am for the second too. |
The current situation is all right with me.
Html::url() already works like that |
@mgrechanik what about controller's |
controller's redirect could redirect to any urls when controller's createUrl is used for a different purpose (creating some very specific set of urls). |
The question is now how bad it is, the question is how to make it better. |
Changing controller's method's signatures to ([$route, $params]) (as people above proposed) won't make it better at all because controller->crealeUrl will differ too much from UrlManager->crealeUrl. As I see the question is mainly about redirect method, is it not? public function redirect($url, $params = [], $statusCode = 302)
{
$url = (empty($params) || !is_string($url)) ? Html::url($url) : $this->createUrl($url, $params);
//var_dump($url);
return Yii::$app->getResponse()->redirect($url, $statusCode);
} |
@mgrechanik well, it's about unifying ALL signatures. Leaving some with different format doesn't make sense. |
I'm fine with unifying the format to be |
What about URL parsing? Should format be changed there as well? |
Why URL parsing is related with this? On Saturday, February 15, 2014, Alexander Makarov [email protected]
|
It's not. Confused it with something else... |
I'm not sure $params = [];
if ($sortBy) {
$params['sortBy'] = $sortBy;
}
if ($limit) {
$params['limit'] = $limit;
}
$this->createUrl(array_merge(['orders/index'], $params)); |
maybe
? |
That is how it works now. |
@mgrechanik |
I'd choose the first signature, because this is most clear (as in the signature itself, documentation and autocompletion). It makes clear the route is required, parameters are optional. Also, these two are complete different types of data to me, so seperating those seems logical. I would unify them all. For Html::a it would make more sense for the second argument to be the string url (result from the routing), as it could also be a manual URL that doesnt have to be routed. |
I vote for Html::a('link', [...params])...and same signature for other methods |
yes i vote to like this format Html::a('link', [...params], [...options])... and all except the reference should not be required |
I'm thinking about |
// helpers
Html::a($text, ['/site/view', ['id' => 100]]);
Html::url(['/site/view', ['id' => 100]]);
// controller
$this->redirect(['/site/view', ['id' => 100]]);
$this->createAbsoluteUrl(['/site/view', ['id' => 100]]);
$this->createUrl(['/site/view', ['id' => 100]]);
// url manager
Yii::$app->getUrlManager()->createUrl(['/site/view', ['id' => 100]]);
Yii::$app->getUrlManager()->createAbsoluteUrl(['/site/view', ['id' => 100]]); |
Why do we need the brackets around params in this case? |
In order to be able to pass an array of parameters w/o doing any merging. |
I think this format requires more typing and is more error prone. |
OK. Implementing |
Fixes #2426: Changed URL creation method signatures to be consistent
It requires more typing this way, is difficult to phpdoc and makes no sense, for example if you just have an url with no params your creating an Array object for nothing, not efficient at all. |
@samdark is |
Yes. |
On my opinion, current way (yii1.x) is good. Thanks |
Fixes yiisoft#2426: Changed URL creation method signatures to be consistent
So we have 2 signatures for URLs. It's either
($route, [$params])
or([$route, $params])
. I think we'd better choose one and use it consistently.The text was updated successfully, but these errors were encountered: