New static method merge() for the Url class

[RubtsovAV]

You can use it like this:

    $url = Url::merge('https://domain/dir/file?query=value#fragment', 'http://anotherschema');
    $this->assertEquals('http://anotherschema', $url->buildUrl());

    $url = Url::merge('https://domain/dir/file?query=value#fragment', '//anotherdomain');
    $this->assertEquals('https://anotherdomain', $url->buildUrl());

    $url = Url::merge('https://domain/dir/file?query=value#fragment', '/anotherpath');
    $this->assertEquals('https://domain/anotherpath', $url->buildUrl());

    $url = Url::merge('https://domain/dir/file?query=value#fragment', 'anotherfile');
    $this->assertEquals('https://domain/dir/anotherfile', $url->buildUrl());

    $url = Url::merge('https://domain/dir/file?query=value#fragment', '?anotherquery=value');
    $this->assertEquals('https://domain/dir/file?anotherquery=value', $url->buildUrl());

    $url = Url::merge('https://domain/dir/file?query=value#fragment', '#anotherfragment');
    $this->assertEquals('https://domain/dir/file?query=value#anotherfragment', $url->buildUrl());

[gsouf]

Looks like it does exactly the same thing as $url->resolve() does

[RubtsovAV]

Yes, but it's a more full implementation of $url->resolve() . Also you not need create Url instance for use that.

With $url->resolve() you can use only uri starting with /

[gsouf]

Creating an instance is not a problem. Adding this method will have 2 things that do the same thing. You will prefer to update the existing one instead of writing 2 identical methods

[RubtsovAV]

May be we can replace Url class to Zend\Uri ? There is a implementation of the PSR, test coverage and all features.

[gsouf]

I dont want to depend on zend, you try to add something in the library that is not really needed.

[RubtsovAV]

But resolve need. If it is not needed now, that's for sure needed in the future.

I create new method, because I not understand how it will work in exists method. I do not want to break something that already works.

But http-curl-client, already depend on zend. Why you don't want use other open source projects? In an extreme case it can just be copied to your library, without creating dependency.

[gsouf]

Unit tests are here to make sure that current behavior wont break, I take care of this, and I also take care of keeping a clean codebase (clean also means no duplicate code), that's why I don't want to add duplicated code with this merge method. I don't think that it will be needed in the future because it's the same as Url::fromString('someUrl')->resolve('otherUrl');, and this tool is mostly used internally by the library, that's why at the current state I don't see a good reason to add this method in the library.

For some reasons I already chose to not depend on an other library for url management, I checked many of them, they had limitations in their design that I didnt want to deal with, they are made for general purpose, and excellent for web project but not for the library. Additionally writing an url class is not so hard that's why the library ships with its own url package.

Now there are 2 things to take in consideration:

• If the resolve method is not fully implemented and if you need a full implementation now (relative pathes are not covered at the moment), feel free to improve it, there are unit tests that will tell you if you broke something, and I will also review it to make sure everything is fine.
• If for your convenience you really want to use the merge static method/function, then I advice you to add it in your project codebase and internally use the resolve method.

PS: about dependencies ; yes the curl http client depends on diactoros for unit tests only, this dependency is not loaded when you install the package from an other project (require-dev is root-only). I'm not against using other open source projects, but I do it wisely because when we start to depend on an other library we literally depend on it, and that can make the future harder if it was a bad choice.

A exemple of the use of an other open source project: The google client depends on 2 other libraries:

• symfony css component, the reason is that it provide something that is hard to make. Parsing xml with css is some kind of extra tool that is not at the heart of the library and I wont have issue when depending on it.
• zend diactoros, it provides a solid implementation of the http message interface, being an interface I dont have reason to write it by myself because it will be "exactly" the same thing. Though, I actually dont want to depend on zend diactoros, (because it's not the only implementation of psr7 and in a perfect world i would leave the choice to the user), but I do it to make the installation of the library a way easier.

[RubtsovAV]

Ok. I understand you. I will try to improve a full implementation of resolve() method. Thank you for such a detailed response.

Do you think it would be better to previously discussed with you such changes. Or be better create the PR, because code more illustrative?

[RubtsovAV]

Do you plan to create implementation of psr for Url class? That would be useful for use it in request for example.

[gsouf]

I didnt plan to implement such url, but making a tool that converts a serps url to a psr7 url would be good.

For the PR, if the work to do it is small, it's not a problem to do a PR, but if there are a lot of work to do to achieve it, you better discuss it before to avoid to spend time on the wrong direction (opening a dedicated issue before making an implementation is a good workflow)

[RubtsovAV]

Ok. Thanks.