show link to pool in history comments #634
Conversation
|
Текущая реализация выглядит так:
|
|
Ссылкой должно быть "#XXXX", не вижу смысла делать ссылкой весь комментарий. Искать #XXXX в комментариях и заменять лучше с помощью |
|
Саша, в этом есть своя логика. Если есть возможность обойтись без регулярок -- то и не надо их использовать, они "тяжелее". Насчёт ссылки на весь комментарий -- я первоначально хотел ссылку вешать на часть комментария, но более громоздкий код получился. А различие чисто вкусовое. Подправил немного код. |
|
Если не хотите использовать регулярки, используйте хотя бы strpos+substr[_replace], а не substr с фиксированным параметром. |
|
Попробовал регулярки -- да, так намного изящнее получилось.
Обновил реквест. |
|
Внёс # внутрь ссылки, скриншот уже не стал менять - а пулл риквест обновил. |
|
Чуть позже будем мерджить эту ветку, а то не хочется мастер мерджить в два других пулл реквеста |
|
Т.е. поправлять больше ничего не надо, просто ждём удобного момента для того, чтобы слить эту фичу в ствол? |
|
да, если Дима одобрит по возвращении. |
|
Честно говоря, мне не нравится ни имя функции history_get_comment_html, ни то, что html-код генерится внутри php-кода, а не в шаблоне. Но я, в принципе, переживу, а если буду страдать, сам потом поправлю. |
|
Не надо страдать, лучше до мерджа PR сделать все максимально хорошо, а не доделывать потом :) Андрей, а почему бы логику замены #ID на ссылку не реализовать в шаблоне templates/history.tpl? А в php-скриптах никаких изменений не делать. |
А разве против? ) Просто когда я писал письмо и предложил обсудить фичу -- разговор как-то быстро заглох "тут и обсуждать нечего, надо сделать и всё". Не обсудили концепт -- я делал по видению, который был у меня самого. Могу объяснить, почему я сделал именно так. Если не считать моего некоторого перекоса делать всё на серверной стороне, то надо понимать, что у меня идеальный вариант находился вот в каком направлении: в комментариях разрешен html, вся разметка находится в базе, php тупо достаёт данные из поля, бекенд тупо отрисовывает. Никаких If'ов и функций вообще не планировалось. И предполагалось, что миграция будет содержать обновление базы, которое добавит разметку, а также проставит корректные номера в анкоре ссылки (потому что на самом деле тот номер пула, который пользователи привыкли видеть после хэштега -- это не внутренний ID пула, а человекопонятный номер) И хотя и кратко -- но это всё было мной описано изначально, возможно тогда на это не обратили внимания, а всплыло только сейчас? Всё, что можно поправить до слияния в ствол -- всё нужно править, давайте обсуждать. И имя функции и чёткое разделение на сбор данных и отображение данных. И вливанием в ствол работа над фичей не ограничится, для это был один из промежуточных коммитов (коммит два -- переделка комментариев на html, шаг три -- сделать отдельный таб для ссылок на задания в ещё не до конца промодерированных пулах). Я просто очень не люблю ждать, поэтому для меня всегда проще договориться о маленькой фиче, быстро выкатить чтобы уже ей можно было пользоваться и тут же начать доводить её дальше. А вообще -- сроки выкатывания фич в продакшн у нас очень быстрые: вот один патч, за которым я посматриваю, до сих пор не влили в транк. |
|
Извините, что влезаю, но если Вы будете писать меньше текста, то нам будет Ира 14 мая 2015 г., 11:34 пользователь Andrey notifications@github.com
Ирина |
|
@irinfox И вы меня извините, что так длинно пишу. |
Это pull request к issue #633