1. Oleg Sychev Avatar Oleg Sychev
  2. POAS Moodle plugins
  3. Moodle Plugins
  4. Issues

Рефакторинг DFA-matcher

Issue #3 closed
Oleg Sychev repo owner created an issue

Originally reported on Google Code with ID 3

Осуществить переработку класса DFA-matcher, заменив switch на наследование.

Т.е. для каждого вида узла (возможно, кроме листьев) должен быть создан подкласс от
общего абстрактного класса, содержащие функции с кодом, описываемым типом этого узла
в функциях nullable, firstpos и т.д.

Это значительно упрощает добавление новых типов узлов, т.к. оно сводится к добавлению
одного класса, а не правки набора switch'ей, разбросанных по разным местам кода.

Reported by oasychev on 2010-09-20 09:07:26 - Blocking: #5

Comments (112)

  1. Oleg Sychev reporter

    ``` Похоже, что это изменение необходимо сделать первым, т.к. блокирует все остальные.

    Переработка предстоит серьезная и, к сожалению, делать ее пошагово (один-два типа узла) будет весьма неудобно, так что возможно придется делать без промежуточного тестирования. Однако она крайне необходима и практически от ее успеха зависит выживание вопроса/реализации алгоритма.

    Такую работу необходимо вести в отдельной ветке (репозитории), поместив изменения в общую ветку после тестирования итоговых результатов. Вы можете делать коммиты в локальную ветку ("вытаскивая" (pull) туда обновления с сервера), либо если для надежности вы хотите "выталкивать" коммиты на сервер, сделайте свой клон на сервере (там есть ссылка) и делайте эти коммиты в него. ```

    Reported by `oasychev` on 2010-09-28 20:23:42 - Labels added: Priority-Critical - Labels removed: Priority-Medium

  2. Oleg Sychev reporter

    ``` В репозитории приведены классы для рефакторинга:

    • preg_node.php - содержит классы с данными об узлах, генерируемые лексером/парсером. Описаны большинство необходимых данных, если надо - добавляйте. Эта часть потребует, главным образом, переработки лексера и парсера. Ну и реализовать поддержку функции match для листов (если будут предложения по изменению ее интерфейса - обсудите со мной).
    • dfa_preg_node.php - соответствующие узлы DFA-matcher. Я написал базовый класс, вам необходимо сделать дочерние. Эти классы связаны с алгоритмом ДКА и будут содержать необходимый для их функционирования код.
      • конструктор - создан так, что его вызов для корня приводит к генерации ДКА-узлов для всех узлов дерева (preg-узлы хранятся внутри). В дочерних классах целесообразно сделать следующее: (1) вызвать родительский конструктор и (2)строго после этого провести необходимую работу по преобразованию дерева (код из convert_tree) если это требуется для данного узла (изменяя, при необходимости, и данные вложенного preg-узла).
      • not_supported - будет вызываться из accept_node или вместо нее, обратите внимание на формат возвращаемых данных в комментарии
      • остальные функции взяты из dfa_preg_matcher и совпадают по именам и код для них следует брать оттуда (но думать! Там есть ошибки или недоделки, например в nullable с квантификаторами типа {,3} и {2,3} я не нашел разницы, хотя первый должен true возвращать а второй - false (если под ним не может быть пустоты еще)). Лучше подглядывать, но переписать заново многие вещи. Код собственно поиска совпадения также должен сильно упростится и стать понятнее, там у вас страшная функция...

    Юнит-тесты придется отредактировать соответственно изменениям в формате данных. Если есть вопросы по созданным мной классам - пишите. В любом случае напишите план работы, когда планируете это выполнять. (Как там, приручили новый Denwer?) ```

    Reported by `oasychev` on 2010-09-28 20:43:49

  3. Oleg Sychev reporter

    ``` Я думаю, что рефакторинг можно провести в две волны: 1. заменить хранение данных с node на дочерние классы от preg_node, эта переделка затронет все и поэтапно едва-ли может быть организована. На этом же этапе код совпадения с листом выделится в функции дочерних классов preg_leaf. Заодно не забудьте добавить в сканер/парсер заполнение данных о начале/конце узлов в строке регулярного выражения. 2. выделять код DFA в дочерние классы, это можно делать по одному-два класса с промежуточным тестированием, т.к. мало на что влияет.

    Создайте серверный клон для рефакторинга, чтобы я мог видеть как идет работа... ```

    Reported by `oasychev` on 2010-09-30 10:00:38

  4. Former user Account Deleted

    ``` Можно было, но уже поздно. Я уже описал все классы, включая классы для ДКА, не вводя их в действие, сейчас планирую приделать их к парсеру убрать соответствующий код из матчера и начать тестировать. ```

    Reported by `Xapuyc7` on 2010-09-30 16:46:19

  5. Oleg Sychev reporter

    ``` В любом случае после тестирования сначала вытолкните все изменения в клон проекта, чтобы я на них посмотрел, а потом уже вытянем их в главный репозиторий. ```

    Reported by `oasychev` on 2010-09-30 16:57:16

  6. Former user Account Deleted

    ``` Хорошо. Я понимаю что здесь неправильное место, но: что за "пару листков" я должне был написать, для участия в конкурсе и куда их потом отдать? ```

    Reported by `Xapuyc7` on 2010-09-30 17:59:16

  7. Oleg Sychev reporter

    ``` "Можно было, но уже поздно" - срок подачи работ на конкурс закончился еще неделю назад. Не последний конкурс - будут еще.

    На будущее для конкурса я буду заводить отдельные issue (есть тип Task) если надо будет вам срочно сообщить... ```

    Reported by `oasychev` on 2010-09-30 18:40:06

  9. Oleg Sychev reporter

    ``` Совет по поддержке \w (\W): она должны использовать функции PHP типа ctype_alpha (и т.д.) вместо списка букв. Так будет учтена установленная локаль, что делает и PCRE. Юникод для \w поддерживается только при особых спец-настройках, так что пока не надо...

    P.S. Не вижу клона с текущим состоянием работы... ```

    Reported by `oasychev` on 2010-10-02 20:30:07

  10. Oleg Sychev reporter

    ``` Обратите внимание на последние изменения и слейте со своей работой. Функция from_preg_node сделана статической, чтобы dfa_preg_matcher мог вызвать ее для корня дерева не создавая перед этим каких-то фальшивых объектов dfa_peg_node (без нее плохо, кто знает какого типа узел окажется корнем дерева?).

    Ее вызов должен запустить преобразование всего дерева с данным корнем. ```

    Reported by `oasychev` on 2010-10-02 20:44:55

  11. Oleg Sychev reporter

    ``` Разместите на сервере свой клон и выталкивайте туда коммиты в процессе работы. Если у меня будет время я посмотрю и, возможно, поправлю какие-то ошибки заранее.

    Только не выталкивайте недоделанный рефакторинг в главный код пока он не закончен, не измерена производительность и я на него не посмотрел. ```

    Reported by `oasychev` on 2010-10-03 18:41:15 - Status changed: `InProgress`

  12. Former user Account Deleted

    ``` Я создал клон и вытолкнул в него недосмёрженный код, когда существующий паралельно код будет приведен в чувство я вытолкну и его. ```

    Reported by `Xapuyc7` on 2010-10-09 13:07:13

  13. Former user Account Deleted

    ``` Функция not_supported вернет ложь для поддерживаемого типа узла, но для неподдерживаемого типа она будет неопределена, т.к. узел неподдерживаемого типа не будет иметь dfa_ версии своего класса и останется узлом вышедшим из парсера. Предлагаю ввести ф-цию с таким же именем в класс preg_nodе, т.к. тип узла непереопределенный для своего движка неподдерживается движком. Ваше мнение? ```

    Reported by `Xapuyc7` on 2010-10-10 21:07:07

  14. Oleg Sychev reporter

    ``` not_supported - в preg_node Не согласен по двум причинам: 1. когда будет написано больше матчеров (NFA, backtracking), непонятно к кому именно будет относится not_supported из preg_node, это ведь общий класс - он в принципе не должен содержать сведений, зависимых от конкретных матчеров; 2. not_supported предназначена для ситуаций, когда тип узла поддерживается но неподдерживаемость обусловлена внутренними параметрами (например вы сейчас не поддерживаете жадные квантификаторы, а даже реализовав поддержку условных подмасок не сможете реализовать условные подмаски с обратными ссылками в DFA варианте, т.к. там обратные ссылки не работают).

    Проблема в том, что отсутствие dfa_preg_node класса может обозначать две вещи: (1) узел не поддерживается; (2) узлу не нужен специфичный для DFA-код (как для многих листьев например). Т.е. по отсутствию dfa_preg_node потомка узла нельзя однозначно сказать, что он не поддерживается.

    Пока (навскидку) лучшем мне кажется вариант ввести дополнительно в dfa_preg_node статический массив поддерживаемых типов узлов (по константам TYPE_...) и проверять его если dfa_версии класса нет, генерируя ошибку если узла в списке нет. Вы всегда можете перед вызовом not_supported проверить наследование объекта функциями типа is_a или is_subclass_of. ```

    Reported by `oasychev` on 2010-10-10 21:20:31

  15. Former user Account Deleted

    ``` Я видимо не совсем правильно поянл смысл этой ф-ции. Кстати в условиях нашего вопроса и его однострочного ответа, независимо от матчера, \A \Z эквивалентны ^ $, стоит ли иметь отдельные подтипы для \A \Z? ```

    Reported by `Xapuyc7` on 2010-10-10 21:47:36

  16. Former user Account Deleted

    ``` Специальный ДКА код не нужен только для листа пустоты и узла подмаски, которые конвертируются. Для всех остальных ДКА код нужен, хотя бы для того, чтобы функция number пронумеровала листья, пройдя через узлы. А если ф-ю convert_tree отнести к узлам, то для подмаски ДКА код понадобится, но для листа пустоты всё равно нет, ф-я будет вызываться для его узла-родителя, превращя альтернативу в квантификатор ?. Это так, к слову. ```

    Reported by `Xapuyc7` on 2010-10-10 21:53:51

  17. Former user Account Deleted

    ``` Поддержка \G затруднена тем что при текущем формате ф-ии match лист не получает информацию об оффсете. Может передавать ему информацию об индексе и оффсете отдельно, а не единую информацию о позиции? ```

    Reported by `Xapuyc7` on 2010-10-10 21:56:39

  18. Oleg Sychev reporter

    ``` \A \Z - проще реализовать единый код в switch для соотв. листа написав несколько case подряд, чем при необходимости в дальнейшем поддержки многострочного режима доделывать поддержку новых лексем. (кстати, матчер может использоваться повторно в других проектах кроме preg если понадобится, у него очень удобный интерфейс)

    Нумерация: по правде говоря я вообще не уверен в ее необходимости (если вы ссылаетесь на узлы по номерам, то должны хранить их копии в каком-то массиве, индексированном по этим номерам?). Есть у меня подозрение, что вместо номеров можно было бы прямо использовать ссылки на узлы. Но окружающий это дело код настолько сложен, что реально разобраться до рефакторинга в этом проблематично. Возможно что нумерация будет просто устранена.

    Насчет \G посмотрю доки когда будет чуть больше времени, php-доки неясны, надо в PCRE доки лезть (http://www.pcre.org/pcre.txt) - они подробнее ```

    Reported by `oasychev` on 2010-10-10 22:10:50

  19. Former user Account Deleted

    ``` Насчет \A\Z я так и сделал. Массив connection теперь будет хранить ссылки на узлы, т.к. теперь они необходимы для определения попадания символа в узел, раньше для этого использовались строки с перечнем символов. Использование ссылок вместо номеров по идее возможно, хотя об этом следует подумать. ```

    Reported by `Xapuyc7` on 2010-10-10 22:20:12

  20. Former user Account Deleted

    ``` Что произойдёт если сравнить ссылки на два отдельных листа, имеющих одинаковые значения? Например два листа с буквой а. Если ссылка1 == ссылка2 даст истину, то использование ссылок вместо номеров не возможно, т.к. приведет к перепутыванию листов и тогда, например, аа сработает как аа* или как аа+, точно не уверен, но и то и то будет неправильно. ```

    Reported by `Xapuyc7` on 2010-10-11 00:00:29

  21. Oleg Sychev reporter

    ``` PHP Мануал почитать нельзя? "Справочник языка", раздел "сравнение объектов". Все зависит от операции сравнения, которую вы будете использовать: == или ===

    А еще несложно провести эксперимент (просто создайте в денвере отдельный от мудла хост и сделайте там тестовую страничку, куда будете вставлять сомнительный PHP-код и смотреть на результаты его работы)...

    \G - момент оригинальный, т.к. offset в том понятии, что используется там, у нас нет вообще (не путайте с внутренним offset в процессе поиска совпадения, это разные вещи). Предлагаю пока реализовать как \A и вписать комментарий с TODO для введения такого параметра вообще (начинать надо с preg_matcher::match) ```

    Reported by `oasychev` on 2010-10-11 11:47:25

  22. Former user Account Deleted

    ``` Я описал классы для используемых в ДКА узлов, пока не вводя их в действие и вытолкнул это в клон. P.S. \G я пока не реализовал никак, TODO в свитче и not_supported даёт истину для такого листа.

    ```

    Reported by `Xapuyc7` on 2010-10-11 23:50:09

  23. Oleg Sychev reporter

    ``` 1. Если какие-то функции имеют одинаковый код для многих классов (например numbering для операций, если она выживет) то стоит иметь их код в родительском классе (аналогичном preg_operator) для всех операций (или листьев, тогда функции в самом верхнем классе будут абстрактными. Отучите себя, наконец, от копи-паста. Это крайне вредная привычка! 2. Я не понимаю разницы между dfa_node.php и dfa_preg_node.php, они же вроде одно и тоже делают. 3. Старайтесь писать комментарии почаще. И это не Си, никаких фокусов с определением констант не нужно - require_once подключает файл однажды, на то оно и once.

    ```

    Reported by `oasychev` on 2010-10-13 21:29:15

  24. Former user Account Deleted

    ``` Я столкнулся со следующей проблемой: символьный класс, допустим, [,(){}\w], по идее, в новой реализации, его нужно представить как [,(){}]|\w, но передавать из лексера в парсер целое поддерево не кажется мне хорошей идей. Можно например передавать "особый лист" preg_leaf_meta subtype = \w, со свойством charset = остальной символьный класс, но опять же, это как-то некрасиво. Ваше мнение? ```

    Reported by `Xapuyc7` on 2010-10-19 18:16:03

  25. Oleg Sychev reporter

    ``` Ну по большому счету такие вещи делаются на стадии между синтаксическим анализом и выполнением - его семантическим анализом называют. Но это добавляет нам лишний обход дерева.

    Возвращать из сканера альтернативу нельзя - это не лексема. Я бы вернул из сканера узел символьного класса, а в парсере превратил его в два если он содержит \w (или \W) при конвертации в expr из листа. Причина такого решения прежде всего в том, что оно соответствует нашей структуре данных дерева, которая просто не позволяет иметь \w внутри символьного класса.

    Либо наоборот, добавить в узел символьного класса флаги наличия \w и \W и вынести его из preg_leaf_meta. ```

    Reported by `oasychev` on 2010-10-19 20:40:24

  26. Former user Account Deleted

    ``` Я осуществил рефакторинг лексера и парсера, они работают на тестах, но в парсере имеется 11 конфликтов. Работоспосбность матчера в целом нарушена. Изменения вытолкнуты в клон. ```

    Reported by `Xapuyc7` on 2010-10-23 03:39:36

  27. Oleg Sychev reporter

    ``` 1. Конфликты явно вызваны отсутствием приоритета у ENDANCHOR. Однако я не могу понять, зачем в новой конфигурации вообще лексемы типа STARTANCHOR, ENDANCHOR, WORDBREAK и т.д. - я бы все их свел к PARSLEAF, какая парсеру разница какого типа лист?! Эта информация должна уже быть заполнена сканером и волновать парсер только при наличии \w внутри символьного класса, но это уже не к грамматике относится, а к коду действий. 2. Почему операнды нумеруете с 1? Может удобнее с нуля все-таки будет? 3. В классе preg_node были добавлены поля $indfirst, $indlast чтобы отмечать позиции символов в тексте - это будет использовано для сообщений об ошибках и в других инструментах помощи при создании выражений. Сканер и парсер должны их заполнять. Сканеру в этом помогает директива %char (см. документацию по JLex, а вот %line я бы убрал - зачем оно?), ну а парсер просто агрегирует, для сложных узлов проставляя от начала первого до конца последнего. Добавьте их заполнение в сканер и парсер. 4. В сканере form_node($name, $subtype = null, $charclass = null, $leftborder = null, $rightborder

    null, $greed = true) {

    $result = new $name; if (isset($subtype)) { $result->subtype = $subtype; } логичнее условие написать "if ($subtype !== null)" т.к. вы задали значение по умолчанию 5. Поддержку unicode properties пока придется организовать через ... вызов preg_match, другого варианта просто нет (хоть это и медленно). 6. Будьте аккуратны в сообщениях о коммитах, следите за языком - вполне возможно вам когда-нибудь придется ссылаться на них устраиваясь на работу.... ```

    Reported by `oasychev` on 2010-10-23 13:19:50

  28. Oleg Sychev reporter

    ``` И еще - в модульных тестах избегайте && в ассертах, чтобы потом не гадать какое именно из условий нарушилось... Лучше отдельные ассерты. ```

    Reported by `oasychev` on 2010-10-23 13:20:45

  29. Former user Account Deleted

    ``` Классы узлов приведены в божеский вид, а лексер и парсер поправлены в соответствии с 1-4. 5 пока не сделано. ```

    Reported by `Xapuyc7` on 2010-10-23 20:12:48

  30. Oleg Sychev reporter

    ``` Еще моменты: 1. Вызывает вопросы find_asserts. 1.1. $this-> в комментах явно утратила актуальность. 1.2. Сдается мне, что если внутри ассерта расположен другой(а кто запрещал?), то внутренний не будет обнаружен вашей функцией. 1.3. Мне кажется что поиск ассертов вполне можно разместить в конструкторе (как и другую работу по однократной обработке дерева) - чтобы не делать лишние обходы и не программировать их. Для этого необходимо обеспечить dfa_preg_node контактом с объектом матчера. Это можно сделать так: заполнять статические поля dfa_preg_node нужной информацией (теми же корнями деревьев), а из матчера потом считать их (поля protected, доступ к ним для матчера через статические функции чтения). 1.4. Как вам идея использовать отрицательные номера узлов для ассертов? Это лучше чем случайно взятое огромное число...

    2. Не делайте код функции сложнее, чем это нужно. Например универсальная реализация обхода дерева для dfa_preg_operator требует foreach, но когда вы программируете конкретные операторы зачастую можно обойтись без цикла.

    Например рассмотрим dfa_preg_node_concat::nullable мне кажется может быть сведена к return $this->pregnode->operands[0]->nullable() && $this->pregnode->operands[1]->nullable(); (только отформатируйте его по правилам, на двух строчках) Аналогичные изменения можно сделать в других местах.

    3. Также в dfa_preg_node_concat::followpos (и не только в ней) вы зачем-то скопировали код родительской функции вместо вызова ее т.е. parent::followpos(...) Перестаньте, наконец, копировать код...

    4. dfa_preg_node_finite_quant::nullable - разве a+ у вас не преобразуется в aa* ? Также опечатка в $result в этой функции.

    5. Не забывайте что not_supported должна возвращать имя строки с названием того, чего не поддерживается (для доступа через get_string), наподобие использованных сейчас в preg_matcher::accept_node.

    6. (даю новую нумерацию чтобы продолжать разговор без двусмысленности) если понимаете как сделать unicode properties - сделайте, там элементарно. Сканер выделяет ее название, а узел потом генерирует новое регулярное выражение для совпадения с одним символом... ```

    Reported by `oasychev` on 2010-10-25 21:34:37

  31. Oleg Sychev reporter

    ``` 1.3. пока отменяется - статические переменные не годятся, т.к. в программе может присутствовать много объектов-матчеров сразу. Пока думаем как обеспечить двустороннюю связь между матчеров и иерархией узлов - она нужна во многих случаях... ```

    Reported by `oasychev` on 2010-10-25 21:45:27

  32. Oleg Sychev reporter

    ``` Замечания по preg_nodes.php 7. preg_leaf_meta::match содержит две очевидных проблемы 7.1. в случае точки следовало бы все же проверить, существует ли символ в указанной позиции - даже если dfa_matcher такой ситуации не создаст, другие могут - узлы то универсальные (кроме того, лучше заложится на общий случай и помнить что по умолчанию точка не совпадает с переводом строки) 7.2. будьте осторожнее с присваиванием длины, особенно отрицательных чисел. можете обнаружить очень странное поведение если $this->negative истинно, а символ не принадлежит слову...

    8. Совершенно не понял реализацию SUBTYPE_WORDBREAK. Чтобы определить границу слова надо сравнить два соседних символа (напоминаю, позиция в строке с точки зрения ассертов находится МЕЖДУ символами), один из них должен принадлежать слову, другой нет. ```

    Reported by `oasychev` on 2010-10-25 21:57:48

  33. Oleg Sychev reporter

    ``` Как идет продвижение по пунктам 1-8 (там вроде нигде не сложно) и восстановлению работоспособности матчера? ```

    Reported by `oasychev` on 2010-11-05 22:23:38

  34. Former user Account Deleted

    ``` Медленно, буду ускорятся. ```

    Reported by `Xapuyc7` on 2010-11-09 15:10:56

  35. Oleg Sychev reporter

    ``` Если говорить серьезно, то успешное проведение рефакторинга - это вопрос выживания написанного кода. Если он пройдет - код сможет развиваться дальше, иначе это тупик который не сможет сравниться с существующими системами матчинга.

    А к декабрю будет много семестровок... Я бы в этой ситуации не слишком медлил... ```

    Reported by `oasychev` on 2010-11-11 21:56:12

  36. Former user Account Deleted

    ``` Восстановлена работоспособность buildfa и её подфункций. Изменения в клоне. ```

    Reported by `Xapuyc7` on 2010-11-14 14:21:11

  37. Oleg Sychev reporter

    ``` 9 (продолжаю нумерацию, т.к. предыдущие пункты не исправлялись, чтобы нам не потеряться) - мне кажется, что dfa_preg_matcher::is_include_characters необходимо перенести в dfa_preg_leaf (реализация нужна не только для dfa_preg_leaf_charset, но и для dfa_preg_leaf_meta, например буквы могут входить внутрь \w, а внутрь точки входит все вообще). А лучше, наверное, в preg_leaf, функция слабо зависит от DFA. ```

    Reported by `oasychev` on 2010-11-14 19:41:52

  38. Oleg Sychev reporter

    ``` 10. Еще я не понял, зачем print_indent является статической функцией. Она бы автоматически наследовалась и вызывалась через $this везде. Это отладочная печать, так что гоняться за производительностью тут тоже странно... ```

    Reported by `oasychev` on 2010-11-14 19:49:49

  39. Oleg Sychev reporter

    ``` Рад видеть, что compare оживает (и приобретает вразумительный размер). Но есть несколько вопросов (продолжаю нумерацию до появления отчета об исправлении предыдущих пунктов): 11. Вы не используете параметр $length, который возвращает вам лист, осуществляя сравнение между тем даже сейчас существуют узлы двух типов: символьные (дают длину 1) и ассерты (дают длину 0). По идее, его использование позволило бы убрать логику, отделяющую листы-ассерты от символов из compare и упростить ее, хотя потребует некоторого изменения алгоритма. 12. Логику обработки "больших" ассертов (строки 411 dfa_preg_matcher.php и далее) я бы вынес в функцию узла dfa_preg_node_assert. Сейчас она кажется тривиальной, но стоит нам добавить поддержку других видов ассертов, как сильно усложнится. Я бы выделил работу с ассертами в единую функцию, находящуюся в узле-ассерте; которая бы уже разобралась с типом ассерта и вернула все необходимые данные. Даже если сейчас это одна строчка будет для положительных впередсмотрящих ассертов. 13. Строки 471-475 dfa_preg_matcher.php вызывают вопрос, т.к. аналогичный код у вас есть теперь в preg_leaf_charset::character. Думаю, следует этот код из compare изъять и заменить вызовами character.

    P.S. Как проходят общие юнит-тесты на матчинг? (тесты на обработку ошибок можете пока игнорировать, я ее в любом случае переделаю после интеграции рефакторинга) ```

    Reported by `oasychev` on 2010-12-13 22:17:53

  40. Oleg Sychev reporter

    ``` Какие планы на прогресс в рефакторинге? Немного же осталось... Главным образом убрать из compare логику, связанную с ассертами, перейдя на возвращенную длину совпадения; и поисправлять мелкие замечания, пронумерованные здесь.

    Когда сможете заняться? ```

    Reported by `oasychev` on 2011-01-13 16:09:09

  41. Former user Account Deleted

    ``` В течение недели точно, может быть раньше. Из compare не нужно убирать логику связанную с асертами, т.к. она обрабатывает "большие" асерты (?=.*b), а не листы-ассерты, таким образом, нужно добавить поддержку листов-асертов (сейчас её нет) в дополнении к обработке "больших" асертов и исправить пронумерованные замечание. ```

    Reported by `Xapuyc7` on 2011-01-15 19:13:14

  42. Oleg Sychev reporter

    ``` Я вот думаю, а что будет если рассмотреть "большие" ассерты как форму "листа"? Ведь вы практически трактуете их как лист, имеющий внутреннюю, сложную процедуру проверки, для главного автомата большой ассерт скорее лист, чем операция, связывающая другие листы автомата. Мне кажется compare существенно бы упростилась в этом случае.

    Но для этого, видимо, надо внести в лист функции связанные с поиском следующего символа. ```

    Reported by `oasychev` on 2011-01-15 19:37:06

  43. Former user Account Deleted

    ``` "Большой" ассерт может содержать вложенный в него подрегекс любой сложности, и процедура для поиска совпадения это функция compare с предварительным построением ДКА, конечно можно в функции match листа создавать объект dfa_preg_matcher и с его помощью осуществлять сранение строки с подрегексом ассерта, но это не влезет в формат функции match. Ведь в случае "большого" ассерта помимо факта совпадения и его длины, так же, необходимо знать символ который нужен на следующей позиции, вариантов то будет много, а функция next_character для этого не подойдет, ведь следующий символ будет определен в зависимости от предыдущих усмпешно принятых, а next_character выдает его безусловно. При этом большая часть логики обработки "большого ассерта в compare останется, она связана с определением следующего символа в зависимости от следующего символа требуемого ассертом, следубщего символа требуемого основным регексом, длины совпадений с ассертом и с основным регексом, начальной позиции совпадения с ассертом и полноты совпадения с ассертом. По этому, я считаю целесообразным сохранить текущую обработку "больших" ассертов. ```

    Reported by `Xapuyc7` on 2011-01-15 21:26:24

  44. Oleg Sychev reporter

    ``` Мне не нравится наличие логики обработки ассертов в compare - введение 4-х типов ассертов может усугубить ситуацию, сделать ее очень сложной. А введение условных подмасок и прочих возможностей - тем более. Предпочел бы перенести такие функции в узлы.

    Подумаю над переносом поиска следующего символа в узлы как только смогу. ```

    Reported by `oasychev` on 2011-01-16 21:07:49

  45. Oleg Sychev reporter

    ``` Касательно обработки больших ассертов: идеальным решением было бы найти или составить алгоритм "слияния" двух автоматов (основного и ассертового) в один. Тогда все проблемы решились бы, без этого решение задачи о следующем символе в общем случае страшноватенькое (ну найдем мы для ассерта, а он может противоречить основному регексу). Надо обоим подумать/поискать. По идее проблема слияния не так уж и сложна. Искать только неудобно - слово merging применительно к конечным автоматам дает прежде всего кучу ссылок на слияние состояний автомата. ```

    Reported by `oasychev` on 2011-01-19 23:58:25

  46. Oleg Sychev reporter

    ``` Ну вы хоть на каникулах рефакторинг доделаете? ```

    Reported by `oasychev` on 2011-01-29 16:05:46

  48. Oleg Sychev reporter

    ``` Ну и? С понедельника начинается семестр... ```

    Reported by `oasychev` on 2011-02-04 20:13:59

  49. Former user Account Deleted

    ``` Замечания в целом исправлены, кроме: 1.2 1.4 отложил до реализации ассертов. 1.3 т.к. отменено 4. Теперь а+ поддерживается непосредственно, без преобразования к аа* 7.2. Непонял, я нигде не присваиваю отрицательной длинны. 9. т.к. is_include_character нуждается в более основательной переработке, в нынешнем виде она не учитывает ситуация когда два СК перекрываются лишь частично [abc] [bcd] 12. Вместо этого будет слияние автоматов и будет при\перед реализации остальных ассертов. Осталось сделать функцию преобразования дерева для {} и отладить взаимодействие buildfa и compare тесты на две этих функции сразу глючат.

    Изменения в клоне.

    ```

    Reported by `Xapuyc7` on 2011-02-07 01:15:27

  50. Oleg Sychev reporter

    ``` 1.2 - ладно, откладываем чтобы видеть нормальную структуру - отдельное issue создам - но напишите пока юнит-тест на ассерт внутри ассерта чтобы не забыть, пускай пока он не проходит... 9 - однозначно отдельное issue, причем у меня есть сильное подозрение что функция character в листе - неправильный подход. Нужно оперировать множествами символов, в т.ч. и для определения следующего: представьте себе ситуацию, где в основном стоит \W (не символ слова), а действующий в этой точке ассерт запрещает наличие там '#'. Пока опять же составить неработающий юнит-тест. .... короче 14. preg_nodes.php строка 326, сообщение странное - причем тут backref? 15. составить юнит-тесты (пока неработающие) для указанных выше случаев: ассерт в ассерте, и "не символ слова" но "#" не подходит. Создайте специальный класс для тестовых ситуаций на еще нереализованную функциональность (testfuture например), чтобы отличать их от ошибок в готовой программе... 16. когда сможете закончить отладку? ```

    Reported by `oasychev` on 2011-02-07 18:04:49

  51. Oleg Sychev reporter

    ``` 17. в сканер/парсер ввести поддержку обратных ссылок - backref, с выдачей сообщения о том что они не поддерживаются DFA-алгоритмом ```

    Reported by `oasychev` on 2011-02-07 18:16:18

  52. Oleg Sychev reporter

    ``` По 9 - переносу is_include_characters, я видел одну из целей рефакторинга в избавлении класса матчера от статических функций (которые реально принадлежат не ему). Так что существующий код я бы все-таки перенес, а его улучшение отнес к только что созданному issue 22. ```

    Reported by `oasychev` on 2011-02-07 18:26:57

  53. Former user Account Deleted

    ``` Восстановлена работоспособность цельного матчера, кроме квантификаторов {} 9. is_include_character это подфункция followposU для символьных классов, нужно переносить еще и половину followPos, точнее говоря удалить и сделать в другом месте заново, потому что неправильна не только реализация (не учитывает включение точкой всего и т.п.), но и сама идея, ведь помимо включения листы могут перекрываться частично (например [abcd] и [cdef]), а такая возможность этой реализацией даже не предусмотрена. Это потребует придусывания нового способа и серьезной отладки. Отдельное issue, ИМХО. А переносить неправильный код я смысла невижу. 14. Ни при чем, мне надо меньше копипастить. Исправил. 15. Составил. 16. Осталось только 17 и 18, Сделаю в ближайщие дни, до понедельника, наверное, а то пора уже и к ассертам приступать. 17. Еще нет. Ввожу новый пункт: 18. Сделать преобразование дерева для квантификаторов {} и восстановить их работоспособность. ```

    Reported by `Xapuyc7` on 2011-03-11 00:00:15

  54. Oleg Sychev reporter

    ``` Рад видеть, что работа матчера налаживается.

    9 Я почему за перенос - неплохо бы изолировать кривой код в нескольких конкретных функциях, тем более если он перемешан в followposU с нормальным. Тогда, если написать юнит-тесты на эти функции, его выправление будет гораздо удобнее и проще. ```

    Reported by `oasychev` on 2011-03-11 17:12:47

  55. Former user Account Deleted

    ``` 9. Функция followposU так же содержит кривой код, однако она относится к матчеру и её никуда не перенесеш. функция is_include_character может и вовсе не пережить фикса, если для правильного фикса будет избран иной способ, поэтому я не вижу смысла в переносе, 17 и 18 готовы, работоспособность матчера полностью восстановлена, изменения в клоне. ```

    Reported by `Xapuyc7` on 2011-03-13 17:55:33

  56. Oleg Sychev reporter

    ``` ОК. Посмотрю в ближайшие дни...

    P.S. Честно говоря не вижу ничего плохого, чтобы тесты на будущее запускались вместе с остальными: по файлу будет видно что это они, зато их fail'ы будут напоминать о необходимости доделать ```

    Reported by `oasychev` on 2011-03-14 00:43:05

  57. Oleg Sychev reporter

    ``` Одно замечание по юнит-тестам. В ассертах (собственно юнит-тестовых, не из регулярных выражений) не обязательно писать == и конкретные значения. Возможны другие формы проверки.

    Например если есть множество возможных вариантов следующего символа, значит надо проверить что символ попадает в это множество (например через функцию, проверяющую есть ли он в строке или его тип - цифра, буква и т.д.). Это лучше чем писать конкретные значения под существующий алгоритм или зажимать тестовые выражения до одного возможного символа. Я замечал такие правки в последних коммитах. Поэтому:

    19. Универсализируйте тесты, так чтобы проверки проверяли соблюдение условий, а не конкретные значения в тех случаях если возможно несколько правильных ответов. Если существующих маловато, добавьте таких универсальных тестов... ```

    Reported by `oasychev` on 2011-03-14 19:32:08

  58. Oleg Sychev reporter

    ``` И еще - после всей работы вытяните и смержите последние изменения из основного репозитория, чтобы мне было легче вытягивать ваши. ```

    Reported by `oasychev` on 2011-03-14 19:34:08

  59. Oleg Sychev reporter

    ``` Это не совсем в тему, но близко: рефакторенный вариант матчера стоит подать на смотр-конкурс студенческих работ см. http://www.vstu.ru/blocks/news/more/20110305/smotr-konkurs-2011.pdf

    Нужно в марте написать страничку тезисов и отредактировать ее со мной. И подготовить презентацию/программу к демонстрации позже на самом смотре. ```

    Reported by `oasychev` on 2011-03-16 16:21:52

  60. Former user Account Deleted

    ``` пункт 19 выполнил, смержил, вытолкнул в клон. Черновой вариант тезисов сделаю завтра-послезавтра и сразу же отправлю вам. ```

    Reported by `Xapuyc7` on 2011-03-21 23:16:37

  62. Oleg Sychev reporter

    ``` Текст надо сделать более связанным и логично построенным, без искусственных фраз. Примерный план: 1. ЗАЧЕМ это нужно было сделать - есть regexp который умеет подсказки но делает это криво и неудачным методом - указать проблемы - и есть preg который пользуется стандартными функциями, но они не позволяют находить частичные соответствия и делать подсказки, уточните какую часть вопроса делали именно вы - еще написать что должно дать: частичное совпадение (уточнить, почему оно полезно) и следующий символ 2. ПОЧЕМУ выбран метод ДКА - кстати, расшифровать при этом, при первом упоминании пишите полностью и в скобках аббревиатуру 3. 1-2 абзаца о деталях реализации - поддерживаемых операциях, волне для определения следующего символа, классах, у вас выигрышная тема по мотивам рефакторинга - теперь структура программы гораздо лучше 4. выводы - как может использоваться и что еще надо доделать

    Избегайте искусственных фраз типа "У данного класса матчера есть 1 аналог" - кстати это аналог не класса, а вопроса в целом. В начале говорите что-то типа "существующий аналогичный вопрос regexp не может быть использован потому-то и потому-то". Когда говорите о производительности, приведите время прохождения юнит-тестов - Moodle вроде бы его выдавал - сравнение с генерацией формы мало что дает (можно сказать о веб-странице пользовательского интерфейса). Про НКА и рекурсию не упоминайте, ваша работа - ДКА. Но сканер/парсер можете упомянуть, вы много для них сделали.

    P.S. Как только разгребу серию неотложных задач - посмотрю код. Я уже прикидываю пару вредных тестов к поиску следующего символа. Например выражение ",\b[?!abc]" и строку ",e" - он должен выдать как следующий возможный символ a (b,c) но не ? или ! (и наоборот "a\b[?!abc]" со строкой "a." допускает только знаки препинания). Если есть время - допишите в юнит-тесты. ```

    Reported by `oasychev` on 2011-03-28 19:56:03

  63. Oleg Sychev reporter

    ``` Меня удивляет поведение preg_leaf_assert::character. Ну ладно конец строки - там можно выдать сообщение, что ответ должен закончится (кстати, если это не символ а сообщение пользователю - значит нужно использовать get_string). Если "следующий символ" должен быть началом строки - ситуация вообще странная (если это только не самый первый символ, но тогда совпадения вообще не было). Но если это граница слова - можно выдать конкретный символ... (правда, для этого нужно знать предыдущий).

    Вообще при определении следующего символа для листа-ассерта необходимо учитывать следующий нормальный (не-ассерт) переход - например "^a" даст следующий символ "а", а не начало строки - а для границы слова - еще и предыдущий... Сделать это character в одном узле вряд-ли сможет - для этой логики надо другое место искать... ```

    Reported by `oasychev` on 2011-03-31 13:36:59

  64. Oleg Sychev reporter

    ``` Еще проблема - наследование dfa_preg_node_infinite_quant от dfa_preg_node_finite_quant. Лучше бы наоборот, если уж очень надо, т.к. в соотв. классе pre_nodes.php больше данных у конечного квантификатора (правая граница), чем у бесконечного... ```

    Reported by `oasychev` on 2011-03-31 13:46:16

  65. Oleg Sychev reporter

    ``` Короче примерно такой план работ: 20. Вам восстановить работоспособность dfa_preg_matcher::for_case_insensitive() - потому как она пытается сделать strtolower/strtoupper над узлами типа dfa_preg_nodes вместо строк - и добавить общие юнит-тесты на case-insensitive matching чтобы такие глюки больше не проползали... Видимо, необходимо добавить параметр caseinsensitive для preg_leaf при матчинге (или создании узла? если нужно и для character то при создании узла запомнить в поле класса). А костыль в виде for_case_insensitive() - убрать... 21. Переделываете наследование dfa_preg_node_infinite_quant и dfa_preg_node_finite_quant - см. выше. 22. Желательно также добавить в парсер поддержку TYPE_LEAF_RECURSION и TYPE_LEAF_OPTIONS, чтобы нормально генерировались сообщения о том, что они не поддерживаются... 23. Я загружаю свои поправки чтобы восстановить работоспособность accept_regex/accept_node и объясняю что нужно сделать поверх их.

    Ну а работу над генерацией следующего символа для простых ассертов оставляем до выправления всей генерации следующего символа...

    ```

    Reported by `oasychev` on 2011-03-31 14:43:06

  66. Oleg Sychev reporter

    ``` Сейчас две срочных работы: а - поправить тезисы - найти меня чтобы мы поправили и подписали окончательный вариант можно в понедельник (18-30), вторник (13-30..14-00) или среду (но там тяжело, разве до занятий); б - восстановить работу матчера в режиме нечуствительности к регистру - сейчас он вообще вылетает, упустили при тестировании этот момент - причем уже сделать по человечески, с новой структурой классов это несложно - чтобы я мог вытянуть изменения в основную ветку и загрузить поверх них те правки, что я начал делать в механизме обнаружения ошибок. ```

    Reported by `oasychev` on 2011-04-02 15:17:29

  67. Former user Account Deleted

    ``` Восстановлена регистронечувствительность и исправлено наследование классов квантификаторов. Изменения в клоне. ```

    Reported by `Xapuyc7` on 2011-04-19 12:49:21

  68. Former user Account Deleted

    ``` В парсер (точнее говоря в лексер) добавлена поддержка листов опций и рекурсии, изменения в клоне. ```

    Reported by `Xapuyc7` on 2011-04-20 06:07:59

  69. Oleg Sychev reporter

    ``` Вам не кажется, что лучше передавать регистрочуствительность при создании листа и запоминать в свойстве, чем каждый раз передавать в match? Я что-то не могу представить себе ситуацию, в которой один объект листа делал бы и регисторочуствительный и регистронечуствительный матчинг....

    P.S. Надо уже срочно рисовать презентацию к докладу. У вас нет опыта, поэтому надо будет пару раз встретится - поправить. Ориентируйтесь на 7-10 плакатов (учитывая титульный). Примерно 3 раздела: а) что это позволяет сделать - кратко что такое регулярные выражения и зачем нужны, конкретные примеры использования вопроса, включая пометку правильных и неправильных частей и выдачу подказки - подробно показать 1-2 примера использования; б) внутреннее устройство - как работает, наприсуйте автомат, матчинг и волну по нему, потом диаграмму классов; в) что реализовано, примеры скриншотов Главное показать что она позволяет делать и как это полезно. Куча текста на плакатах не должна быть (кроме титула) - перечни, схемы, рисунки. Текст вы будете говорить...

    ```

    Reported by `oasychev` on 2011-04-20 15:38:58

  70. Oleg Sychev reporter

    ``` Я создал клон для того, чтобы можно было вытолкнуть изменения в рефакторинге без затрагивания основного репозитория пока. Вытяните изменения оттуда.

    Я начал работу по рефакторингу генерации сообщений об ошибках: отрефакторил accept, теперь принятие осуществляется функциями узлов конкретного матчера - это дает им возможность активно принимать во внимание параметры (например отказываемся только от нежадных квантификаторов) и избавляет от switch. Соответственно претерпел изменения процесс построения дерева - распределение обязанностей между классами теперь четче, и я вынес общий код из dfa. Выявились проблемы и задачи.

    Вам сейчас необходимо сделать следующее: 24. Восстановить работоспособность на юнит-тестах из testquestiontype.php - они ведут себя довольно странно (нарочно оставил одну отладочную печать). Такое впечатление, что где-то при преобразовании квантификаторов происходят странные вещи - посмотрите, во что превратился узел в тесте на альтернативу например. Их необходимо отладить. Непонятно, является ли это следствием рефакторинга или происходило и раньше, т.к. тесты на матчинг проходят успешно... 25.accept_end пытается перезапустить преобразование дерева (from_preg_node), хотя к моменту ее вызова оно уже произошло, необходимо этого избежать (обратите внимание, что создание объекта dfa_preg_node_concat тоже запустит этот процесс если старое дерево будет присутствовать в операндах). Необходимо преобразовывать только новые узлы, а старое дерево добавить после преобразования. 26. прописываете функцию accept в классах dfa-узлов. Она должна вернуть истину/ложь и заполнить поле rejectmsg строкой (полученной через get_string) с указанием вида узла, приведшего проблеме (например не просто "квантификатор", а "нежадный квантификатор").

    Ну и жду вас очень с презентацией - доклад уже скоро, а опыта в подачи работы у вас мало. Самая хорошая работа может провалиться при плохом докладе... ```

    Reported by `oasychev` on 2011-04-30 22:50:23

  71. Former user Account Deleted

    ``` Выкладываю презентацию-сырец. Обоснование зачем это нужно получилось в виде таблиц с текстом, в картинках я это себе не представляю. Пример работы получился с картинками, порядок перехода от состояния к состоянию я отметил цветами, в порядке радуги, я подумал что подписать рядом цыферку будет плохо выдно, диаграмма классов получилась некрасивой, думаю как её улучшить. P.S. что в титульнике писать? P.P.S. пунктами 24-26 займусь в ближайшее время. ```

    Reported by `Xapuyc7` on 2011-05-02 20:24:13

    <hr>

  72. Oleg Sychev reporter

    ``` На этой неделе ко мне можно будет подойти во вторник, в интервале 14-30..15-00 чтобы обсудить презентацию конкретно и на месте поправить. Либо ждать до среды следующей недели и переписываться здесь...

    Добавлю еще пункт: 27. привести в порядок работу с утилитой dot: 27.1. путь к бинарнику dot прописать в виде константы (через define) в начале файла, а еще лучше - в админской настройке вопроса 27.2. через $CFG->dataroot можно получить доступ к каталогу для файлов этой инсталляции Moodle, временные файлы-картинки следует размещать там (предпочтительно создав в каталоге temp подкаталог question\preg). Размещать файлы в каталоге с исходным кодом Moodle нельзя категорически, т.к. на любом приличном веб-сервере запись туда будет запрещена. ```

    Reported by `oasychev` on 2011-05-02 20:41:41

  73. Former user Account Deleted

    ``` Приду 03.05.2011 в 14:30. По поводу пункта 27: это не проблема, сделаю, но сначала объясню, почему изначально сделал так. Это функция отладочной печати, которая будет использоваться в процессе отладки, а отладкой занимаются не на приличном сервере, а на локальном компьютере, на приличный же сервер попадет уже отлаженная версия, которая не будет заниматься отладочной печатью.

    ```

    Reported by `Xapuyc7` on 2011-05-02 21:03:56

  74. Oleg Sychev reporter

    ``` Я понимаю насчет отладочной печати, но сделать его грамотно не сложно, зато полезно: ошибка может проявиться на сервере, на реальном вопросе. Или просто кто-нибудь подключиться к разработке с unix-подобной ОС, и ваш вариант отладочной печати у него работать не будет. Даже такие вещи лучше делать качественно. По хорошему, когда освоим админские настройки, можно будет через них включать/выключать отладочную печать - по идее это несложно. ```

    Reported by `oasychev` on 2011-05-02 21:08:38

  75. Former user Account Deleted

    ``` Сразу после того как сделаю пункт 26. Пункт 24, ошибки исправлены, дело в том что конструктор вызывает append_end(), которая меняет дерево, приделывает к нему в конец лист с признаком конца регекса preg_leaf_meta::SUBTYPE_ENDREG, что и меняло результаты тестов рассчитаных на build_tree, которая этого не делает, считаю разумным в юнит тестах вызывать билдтри, ибо пределывания лишнего узла к концу дерева, в виде нового корня усложнит условия в тестах, без получения какой-либо выгоды. Пункт 25, повторного построения не происходит, запускается from_preg_node обрабатывает только новые узлы, новый корень и лист с признаком конца, остальные узла остаются не затронутыми, так как не являются наследниками preg_node (preg_matcher.php стр 347) Пункт 26, работаю. ```

    Reported by `Xapuyc7` on 2011-05-02 21:48:34

  76. Oleg Sychev reporter

    ``` 24 - в таком случае тест должен использовать dst_root вместо roots[0] для доступа к дереву, т.к. roots[0] теперь заполняет конструктор.

    24.1. Вы смотрели дерево в тесте test_nullable_node_alternative_node где я оставил отладочную печать? Оно мне странным представляется, не великовато для такого выражения? Проверьте, корректно ли там работает преобразование... 24.2. Ошибка, видимо, связана не просто с append_end, а с тем что ENDREG почему-то не является nullable, таким образом отнимая у любого выражения эту возможность. Вы уверены что ENDREG не nullable узел? 25. Защиту от повторной обработки я сам сделал, знаю про ее наличие. Но это страховка - все равно повторный вызов - нехорошо. Можно в append_end сначала заполнить только 1-й операнд (ENDREG) - для foreach все равно с какого числа начинать, вызвать from_preg_node, а только после него заполнять нулевой операнд например. Или, еще лучше, добавить в конструктор dfa_preg_node параметр указывающий, надо ли вызывать from_preg_node для операндов. ```

    Reported by `oasychev` on 2011-05-02 22:05:28

  77. Oleg Sychev reporter

    ``` Вытолкнул в свой клон (oasychev-refactoring-continues) рефакторинг сообщений об ошибках - слейте со своей версией.

    Обратите внимание на новый класс preg_lexem использованный чтобы все узлы дерева могли иметь indfirst и indlast - вы нагородили что-то странное и частичное, даже не посоветовались как заполнить все дерево, хотя решение не сложное, можно было и догадаться.

    Посмотрите, как теперь выводятся сообщения об ошибках парсинга. Они включают в себя текст выражения с помеченными ошибками. Желательно также организовать и вывод сообщений о неподдерживаемых узлах.

    По 27 - путь к бинарнику обязательно в админской настройке, чтобы хранился в БД. И вся прочая работа там - тоже обязательна. Пока пути жестко зашиты в программу, даже второму девелоперу (мне например) сложно пользоваться этим механизмом - не будет же каждый из нас коммитить код со своими путями, каждый раз их меняя?! Это никуда не годится...

    P.S. Ну что, к конференции доделаем рефакторинг? Вроде бы немного осталось... ```

    Reported by `oasychev` on 2011-05-06 22:49:43

  78. Oleg Sychev reporter

    ``` 28. (Срочно!) Ошибка в dfa-матчере: если совпадение начинается не сначала строки (т.е. index_first!=0) то index_last некорректен: он берется не с начала строки, а с начала совпадения. Должно быть легко поправить, но ошибка серьезная, поэтому поправьте с максимально возможной скоростью. И добавьте юнит-тесты на матчинг где совпадение начинается не с начала строки, чтобы таких фокусов больше не возникало... 29. Как вы заметили, конструктор dfa_preg_node получает теперь вторым аргументом ссылку на матчер. Это дает вам возможность решить ту проблему, о которой шла речь ранее - реализовать простые ассерты типа \G \Z и другие узлы, требующие дополнительной информации от матчера. Просто перегрузите конструктор в ассерте и перед вызовом родительского сохраните ссылку на матчер в поле класса.

    P.S. Сегодня вытолкну еще несколько несложных изменений в свой клон, не забывайте мержить их... ```

    Reported by `oasychev` on 2011-05-09 12:37:33

  79. Former user Account Deleted

    ``` Я поправил презентацию, надеюсь на этот раз хорошо. По рефакторингу работаю, закончу эти пункты завтра, или сегодня поздно ночью. Кроме 27-го, там возникли проблемы. 1)с помощью .htaccess запрещен доступ в папку moodledata, отобразить рисункии оттуда будет невозможно. 2)Если сделать путь к графвизу через админскую настройку и БД, то отладочная печать будет работать только в составе прега, но не у отдельного матчера, как например в юнит тестах, разве нет? ```

    Reported by `Xapuyc7` on 2011-05-10 19:54:18

  81. Former user Account Deleted

    ``` Сделано кроме 27,29. Изменения в моем клоне. Всё смёржено. ```

    Reported by `Xapuyc7` on 2011-05-10 22:14:48

  82. Oleg Sychev reporter

    ``` 27 1) показ картинок из moodledata производится с помощью специальных скриптов Moodle для доступа к файлам, а не напрямую. Для примера генерации и показа картинок посмотрите на mod/quiz/reports - в отчете о прохождении тестов генерируется диаграмма статистики, какие оценки сколько студентов получило. 27 2) сомневаюсь, настройки получаются как? либо через глобалку $CFG либо через функции, не вижу что могло бы помешать получить их в юнит-тестах. ```

    Reported by `oasychev` on 2011-05-10 23:35:40

  83. Oleg Sychev reporter

    ``` В accept вместо switch'а лучше смотрелся бы ассоциативный массив мне кажется. Хотя можно оставить и так. Генерацию результирующих сообщений о не принятых узлах сами не смогли переделать аналогично синтаксическим ошибкам, чтобы они включали выражение с подсветкой? ```

    Reported by `oasychev` on 2011-05-11 00:29:46

  84. Former user Account Deleted

    ``` А что насчёт презентации? ```

    Reported by `Xapuyc7` on 2011-05-11 07:20:12

  85. Former user Account Deleted

    ``` Админскую настройку пути к графвизу я сделал, а вот с выводом картинки из moodledata возникли трудности. Насколько мне удалось понять диаграммы статистики генерериуются с использованием класса graph, который рисует диаграммы, но не произвольные изображения. ```

    Reported by `Xapuyc7` on 2011-05-11 16:37:54

  86. Oleg Sychev reporter

    ``` По презентации: волну честно говоря все-таки не видно на рисунке, но главное - диаграмма классов. Лучше уменьшить количество классов (для однотипных - разных операндов, операций - поставить 2 и многоточие между ними), увеличив шрифт - нечитабельно будет с расстояния. И на ней совершенно отсутствуют классы-матчеры и тип вопроса (а по хорошему еще и лексер и парсер тоже классы, но ими еще можно пожертвовать ради величины шрифта, а вот связь между типом вопроса, матчерами и узлами дерева надо показать).

    По классу graph - ага, он эти диаграммы в виде файла не сохраняет. Ладно, тогда надо разбираться в filter/tex - уж он то точно генерирует картинки с помощью внешнего исполняемого файла. Где он их хранит - надо смотреть. ```

    Reported by `oasychev` on 2011-05-11 16:51:04

  87. Former user Account Deleted

    ``` Так однотипные по двое и представлены, для 2-х дервьев прегноде и дфапрегноде. Представить взаимоотношение типа вопроса и матчера легко, а вот взаимоотношения матчера с класса дфапрегноде это очень много линий, он ведь их всех агрегирует, может отдельным слайдом показать вопрос, матчер и агрегацию им узлов дерева? ```

    Reported by `Xapuyc7` on 2011-05-11 17:11:54

  88. Former user Account Deleted

    ``` Тогда и лексер с парсером влезут на 2-й слайд. ```

    Reported by `Xapuyc7` on 2011-05-11 17:12:41

  89. Former user Account Deleted

    ``` Куда показывать агрегацию парсера, лексер и классов дерева? В preg_matcher или в dfa_preg_matcher? Делаю диаграмму классов в два слайда, один с общей и одним квадратиков на все классы дерева, 2-й подробно расписываю дерево, иначе будет нечего не разглядеть, классов уж больно много. ```

    Reported by `Xapuyc7` on 2011-05-11 21:46:31

  91. Oleg Sychev reporter

    ``` Будьте внимательны: 1) ваш новый текстовый редактор вставляет табуляцию - исправьте на 4 пробела; я поправил в основных файлах, вы проверьте юнит-тесты 2) имя коммитера в последних коммитах также довольно странное и едва-ли имеет ввиду вас 3) функция принятия ассертов почему-то забыла вернуть false если не принимает узел - я исправил...

    29 пока отложим т.к. это вообще-то проблема preg_node а не dfa, а ему матчер пока недоступен. Логичнее всего передать ссылку на матчер в лексер и парсер, и передавать им ее в конструктор только тех узлов preg_node, которые реально требуют связи с матчером.

    30) вы могли заметить что я закомментировал большую часть copy_preg_node . Юнит-тесты работают. Можете ли вы придумать контрпример, который бы не работал без этого кода (вы ведь его писали)? Если нет - его удалим...

    Код подчищен и вытолкнут в основной репозиторий - поздравляю вас с окончанием основной фазы рефакторинга. :) Теперь наши основные цели: а) разобраться с нумерацией и выяснить, нельзя ли использовать ссылки вместо номеров б) ассерты всех видов, простые и сложные - обеспечить нормальную работу, в т.ч. при поиске следующего символа

    ```

    Reported by `oasychev` on 2011-05-12 22:26:32

  92. Oleg Sychev reporter

    ``` Заканчиваем рефакторинг (подчистка проблем): 27) для хранения временной картинки используем каталог $CFG->dataroot/temp/dot (который надо создать если его нет),или вместо dot - preg. В filter/tex надо было смотреть не как выводится файл, а просто где он размещается для начала... Если оттуда не показывается простым тегом - попробуйте показать тем способом, что в фильтре. Возможно способ зависит от размещения файла: теги из wwwroot работают, а та функция - из dataroot 29) сможете передать в сканер и парсер ссылки на матчер, так чтобы сканер их добавлял для \G и backreference к данным узла (и соответственно добавить поле в preg_leaf_meta и обратную ссылку)? Только надо строго проконтролировать (в юнит-тестах через ===) что это именно ссылка, а не копия. Через это реализуем \G чтобы она не висела над душой (добавив функцию в матчер которая сообщает нужные данные узлу, причем функция должна быть абстрактной в базовом классе матчера). 30) остается - придумайте пример, когда копирование операндов (а не ссылок на них) критически важно и добавьте его в юнит-тесты. Кроме того, === для объектов дает возможность проверить один это объект или две копии, так что вы легко можете проверить, копирует ли clone операнды или ссылки. 31) MAX_STATE_COUNT и MAX_PASSAGE_COUNT делаем константами класса dfa_matcher ```

    Reported by `oasychev` on 2011-05-19 20:51:48

  93. Oleg Sychev reporter

    ``` Вытолкнул несколько структурных изменений. 29) \G сделал, ему не самом деле пока ничто не поможет т.к. он работает отлично от \A только при нескольких запусках матчера на одной строке, чего у нас не бывает

    К 27) - если сделали вывод дерева через dot, уберите функции текстовой печати дерева

    Давайте заканчивать, у вас там мелочи остались, и переходить уже к ассертам... ```

    Reported by `oasychev` on 2011-05-20 11:11:11

  94. Oleg Sychev reporter

    ``` 32) не забыть переименовать файлы с тестами: должны получиться testlexer.php, testparser.php, testpregnodes.php (для тестов функций листов - match, character и т.д.), testdfamatcher.php и testdfafuture.php ```

    Reported by `oasychev` on 2011-05-24 22:09:43

  95. Oleg Sychev reporter

    ``` Ну вы сделаете наконец 30...32 чтобы мы оставили позади эти мелочи и перешли к дальнейшей работе?

    И насчет ошибки при повторении символьных классов уже создайте юнит-тест и issue на трекере, вы же давно про нее говорили... ```

    Reported by `oasychev` on 2011-07-11 16:37:10

  96. Oleg Sychev reporter

    ``` 30..32 сделать срочно! Они сдерживают коммит изменений в основной код... ```

    Reported by `oasychev` on 2011-09-06 18:28:12

  98. Oleg Sychev reporter

    ``` 31 раз уж мы научились делать админские настройки типа вопроса когда работали с путем к dot, то MAX_STATE_COUNT и MAX_PASSAGE_COUNT логичнее всего сразу сделать админскими настройками вместо констант.

    Кстати проверьте, при сохранении вопроса (и валидации формы) ошибка при слишком большом числе узлов/переходов выдается или нет? И если нет то почему - автомат при валидации не строится или ошибка не доходит до массива ошибок? ```

    Reported by `oasychev` on 2011-10-24 14:17:17

  99. Former user Account Deleted

    ``` 30-32 выполнены. 30. контрпример я придумать не смог, если на всех этих тестах, которые есть, клон работает, то он видимо работает. ```

    Reported by `Xapuyc7` on 2011-11-01 22:46:09

  100. Oleg Sychev reporter

    ``` 33) Сделайте слияние с новым кодом. И соответствующие изменения: $this->errors[] = get_string('toolargedfa', 'qtype_preg'); больше не годится, т.к. массив ошибок теперь хранит объекты (см. preg_error.php и изменения в матчере). Нужно сделать новый класс для ошибки с большим размером автомата. Если возможно, взять из текущего узла дерева при возникновении ошибки координаты ($indfirst, $indlast) и сохранить их в ошибку - с большой вероятностью это будет именно узел, породивший ошибку. Если нет выставить их так, чтобы они давали пустую строку. Возможно придется предпринять меры чтобы передать данные, какой узел дерева вызвал генерацию сверхбольшого графа, но если это возможно то оно должно быть сделано.

    34) В админских настройках должна быть возможность сделать раздел (группу, рамку). Нужно отделить настройки DFA от общих, потому что будут же и настройки NFA и т.д.

    35) Если функции печати дерева текстом на страницу больше не нужны в связи с использованием dot, их следует удалить.

    После этого переходим к кросс-тестингу и исправлению (или обсуждению) обнаруженных им ошибок. ```

    Reported by `oasychev` on 2011-11-02 10:31:38

  101. Oleg Sychev reporter

    ``` Дмитрий, вы там ничего не делаете или не привыкли выталкивать коммиты на сервер по мере выполнения разных задач?

    Сейчас первостепенное значение имеет восстановить работоспособность юнит-тестов (т.е. не повисать на кросс-тестинге), как только этого добьетесь - выталкивайте изменения.

    Потом прочие исправления... ```

    Reported by `oasychev` on 2011-11-20 15:06:59

  102. Oleg Sychev reporter

    ``` 36) Еще обратите внимание - в вашем матчере возникают warning если строка для совпадения пустая. Этого быть не должно... ```

    Reported by `oasychev` on 2011-11-20 18:05:53

  103. Oleg Sychev reporter

    ``` Релиз назначен на эту неделю!

    Если ДКА-матчер не будет готов к релизу - т.е. все намеченные ошибки не будут исправлены - не составляет труда перевести все вопросы, использующие ДКА на НКА и отключить ДКА-матчер в новом релизе. В этом случае новое включение возможно не ранее следующего релиза.... ```

    Reported by `oasychev` on 2011-11-22 18:54:49

  104. Oleg Sychev reporter

    ``` 30 - на самом деле клонировать предки отдельно было необходимо - это можно проверить юнит-тестами. Плохо придумывали контр-пример - в ваших тестах просто не было вреда от отсутствия копирования....

    Только реализовывать это надо было грамотно - путем переопределения клонирования preg_operator. Посмотрите на последние коммиты в клоне 2.1

    37) Функция copy_preg_node считается устаревшей. Ее вызовы должны быть заменены на клонирование узла после слияния с клоном из 2.1 ... ```

    Reported by `oasychev` on 2011-11-24 17:44:57

  105. Oleg Sychev reporter

    ``` Дмитрий, прекратите вредную практику выталкивания коммитов большой кучей сразу! Выталкивайте каждый день что сделали, это несложно...

    Так мне будет легче следить, что у вас происходит - просмотреть несколько коммитов проще, чем работу за неделю разом... ```

    Reported by `oasychev` on 2011-11-24 22:13:39

  106. Oleg Sychev reporter

    ``` Дмитрий, какого именно рода проблемы с \b остаются у вас на данный момент?

    И будьте добры уже переключиться на 2.1 и смержить код - там много изменений. Функции get_clone для листов например уже нет, она заменена на просто clone ...

    Еще совет - у вас там при вызове cross_meta_leafs вставлено клонирование параметров - не проще было в саму функцию первыми строками написать клонирование? ```

    Reported by `oasychev` on 2011-11-27 16:17:20

  107. Former user Account Deleted

    ``` Проблемы с \b заключаются в проваливании тестов, зависаний нет. С этими проблемамаи, их причиной и способом устранения я буду разбираться в ближайшее время, пока еще не успел. Мержингом я занимаюсь, но получаю от смерженного кода странный фатал еррор: unexpected T_SL на строке 23 в файле lang\en\qtype_preg.php, это строчка начинающаяся с <<<<<<, что это и почему глючит? С этими функциями у меня были проблемы с копированием ссылки, там где я хотел скопировать сам объект, поэтому я расставил очень много клонов, потом убрал лишние, там, пожалуй, стоит получше разхобраться, где клон нужен, а где нет. Этим я займусаь при мержинге, заодно с заменой get_clone().

    P.S. Я ненавижу себя за то, что отложил всё это на последний момент. ```

    Reported by `Xapuyc7` on 2011-11-28 12:17:49

  108. Oleg Sychev reporter

    ``` Вы не разрешили конфликты при мержинге. <<<<<< это как раз обозначение границы конфликта...

    Если проблема только в файле языковых строк, возьмите чистый с моего 2.1 клона и скопируйте поверх своего. Вряд ли вы там последнее время что-то изменяли, а мы правили прилично.

    P.S. Релиз уже запаздывает на 3 месяца и его ждут серьезные люди, в т.ч. пара университетов в Австралии/Новой Зеландии. Откладывать далее я его не могу, так что успевайте как хотите - и делайте выводы на будущее... ```

    Reported by `oasychev` on 2011-11-28 12:28:20

  109. Oleg Sychev reporter

    ``` Исправление ошибок относится собственно к issue 39. Это пока выставляю как выполненное. ```

    Reported by `oasychev` on 2011-12-04 12:30:10 - Status changed: `Fixed`

  110. Oleg Sychev reporter

    ``` Вторая часть рефакторинга посвящена переводу на новый формат автомата и будет осуществлена в рамках перехода на единый формат автомата. ```

    Reported by `oasychev` on 2011-12-12 12:44:52 - Status changed: `Done`

  113. Log in to comment
Assignee
–
Type
task
Priority
critical
Status
closed
Component
–
Votes
0
Watchers
0
Jira: the preferred issue tracker for Bitbucket. Join the team!