ezpublish-legacy icon indicating copy to clipboard operation
ezpublish-legacy copied to clipboard

Published 20 hours ago • verified publisher icon ezsystems

EZP-24114: Autolinkoperator add support to link www url wihout protocol.

Open tompsonx opened this issue 9 years ago • 6 comments

Add support for text which is started from www. without protocol. For example change text "www.ez.no" to link tags.

tompsonx avatar Mar 04 '15 13:03 tompsonx

-1 without ini setting disabling this feature by default (Edit: This is required to retain BC)

brookinsconsulting avatar Mar 04 '15 16:03 brookinsconsulting

I added ini setting disabling this feature by default.

tompsonx avatar Mar 05 '15 00:03 tompsonx

Hello @tompsonx

Thank you for you prompt response.

We have some additional comments but first would you create a jira.ez.no issue ticket and share the link here in the pr?

brookinsconsulting avatar Mar 05 '15 00:03 brookinsconsulting

Thank you for your constructive comments. I rebuilt a little code. What do you think about such modifications. I added static properties, changed preg_replace to preg_replace_callback and added parameter for remove protocols from text.

I create jira issue: EZP-24114.

tompsonx avatar Mar 07 '15 17:03 tompsonx

Link to issue ticket, https://jira.ez.no/browse/EZP-24114

brookinsconsulting avatar Mar 08 '15 09:03 brookinsconsulting

Hello @tompsonx

Apologies for so many individual refactoring requests / comments.

We are striving to ensure that if this pull request is merged, it is done only when the code is of the very best possible quality.

In eZ Publish Legacy we are striving to ensure BC, strict code standards and reliability. In general these days we are actually trying very hard to not make large or unnecessary changes to the code.

For these reasons and (specifically) because of your regex changes. I think for this to be considered for merging we would prefer you also provide extensive unit test coverage of this class covering all previous / existing and new use cases.

Functions that would need to be tested would be those you added and modified. The goal is to make sure they behave the way they are supposed to in each unique use case (variable input use cases, and by our mark, there are quite a few which will need to be written).

Unit test are using phpunit (you can start by looking at tutorials online). You can find documentation on how to run legacy unit test here: http://www.ezpedia.org/ez/testing_ez_publish_test_system

You can add new tests in: https://github.com/ezsystems/ezpublish-legacy/blob/master/tests/tests/kernel/common/ezautolinkoperator_test.php

Cheers, Brookins Consulting

brookinsconsulting avatar Mar 08 '15 10:03 brookinsconsulting