Рефакторинг 2.2: восстановить работоспособность матчеров
Issue #80
closed
Originally reported on Google Code with ID 80
Новый цикл разработки ведется в клоне oasychev-preg-22 Пока код в нем работает под
Moodle 2.1
В качестве начала цикла разработки Preg 2.2 был проведен небольшой рефакторинг, устранивший
некоторые из накопившихся проблем:
# код матчеров вынесен в отдельные каталоги
# данные о совпадении вынесены к специальный класс, теперь функция match_inner возвращает
объект этого класса - требует восстановления;
# вместо индекса последнего символа используется длина совпадения - требует восстановления;
# классы матчеров и ошибок получили префикс qtype_ в соответствии со стандартами Moodle
Авторам матчеров - восстановить работоспособность матчеров по пунктам 2 и 3 главным
образом и кросс-тестов. И отписаться здесь.
Валерий, кросс-тесты не требуют обязательной переделки данных тестов сразу - я добавил
функцию from_length_to_last_index (правда тестировать не на чем). Кросс-тестер пока
должен учитывать возможность поступления как индекса последнего символа, так и длины
совпадения в качестве expected и вести себя соответственно, при необходимости переводя
одно в другое.
Можно будет подумать о функции сравнения результатов совпадения в классе, но сначала
обсудите ее со мной.
Reported by oasychev on 2011-12-26 06:54:34
Comments (36)
-
Account Deleted
-
``` Почему-то перестали нумероваться подмаски, maxsubpatt всегда равен нулю... ```
Reported by `vostreltsov` on 2011-12-29 18:01:14
-
``` С подмасками ошибся, но есть другая проблема: Fatal error: Class 'qtype_preg_accepting_error' not found in Z:\home\moodlestable\www\question\type\preg\preg_regex_handler.php on line 225
Очень странно... ```
Reported by `vostreltsov` on 2011-12-29 18:07:43
-
Account Deleted
``` Мне кажется или это второе сообщение об одной и той же ошибке? :) ```
Reported by `Xapuyc7` on 2011-12-29 18:45:44
-
Account Deleted
``` Вытолкнул восстановленный ДКА, проверка затруднена глюками НКА. Вышеописанная ошибка возникает из-за qtype_reg_... это так и небыло исправлено. ```
Reported by `Xapuyc7` on 2011-12-30 15:48:46
-
reporter
``` Ну так попробуйте исправить и посмотрите, исчезнет ли ошибка. Если выправиться - сделайте коммит и вытолкните.
Проверка может быть затруднена проблемами кросс-тестера, но не НКА как такового... ```
Reported by `oasychev` on 2011-12-30 16:10:24
-
Account Deleted
``` Пробовал исправить, ошибка исчезает. Проблемами именно НКА, он вызывает несуществующую функцию и вылетает фаталка. ```
Reported by `Xapuyc7` on 2011-12-30 16:31:45
-
reporter
``` Так что вам мешает тогда самому это исправить и сделать коммит? А НКА получим с НКА...
P.S. Валерий, вы тогда сливаете изменения Дмитрия и выталкиваете окончательный вариант с работающими матчерами... ```
Reported by `oasychev` on 2011-12-30 16:35:47
-
``` НКА практически восстановлен, но я предлагаю небольшие изменения, которые сильно облегчат жизнь НКА: 1) устанавливать length по умолчанию не -1, а 0. index_first же оставить по умолчанию -1. 2) хотелось бы в случае отсутствия совпадения вообще оставлять index_first[0] = -1, а не выходящим за пределы строки. ```
Reported by `vostreltsov` on 2011-12-31 09:30:49
-
``` Точнее даже лучше делать length[0] = 0, а остальные -1. ```
Reported by `vostreltsov` on 2011-12-31 09:39:03
-
Account Deleted
``` На мой взгляд, первый символ установленый после конца строки имеет больший смысл, нежели -1. В изначальном варианте индекс=-1, это был индекс последнего совпавшего символа, и он был наполнен смыслом -- строка еще не началась, а совпадения уже нет, теперь же первый индекс, и он должен быть после конца строки -- строка закончилась, а совпадения еще нету. С длинной 0 согласен, а факт совпаденияможно же установить по изматч. Хотя всё это не принципиально. ```
Reported by `Xapuyc7` on 2011-12-31 10:39:48
-
reporter
``` Я против нулевой длины при отсутствии совпадения потому что 0 - вполне корректная длина при наличии совпадения.
index_first можно выставлять -1 - собственно функция qtype_preg_matching_results::invalidate_match так теперь и поступает если вы посмотрите на код.
Есть предложение не передавать is_match параметром конструктора, а вычислять на основе full и length[0]. Можно тогда сделать ее функцией. Ваше мнение?
P.S. Всех с наступающим! Успехов в девелопинге, учебе и на прочих фронтах! ```
Reported by `oasychev` on 2011-12-31 16:52:51
-
``` Я согласен, что 0 - корректная длина, но предустановленная length[0] == -1 усложняет жизнь при матчинге, появляется куча лишних проверок условий и в без того огромной функции. Ситуация отсутствия совпадения обрабатывается отдельно ПОСЛЕ матчинга, почему бы его там не выставить в -1?
По поводу is_match согласен. ```
Reported by `vostreltsov` on 2012-01-01 19:24:58
-
reporter
``` Гм. А откуда такие проблемы с -1 берутся? И чем 0 гораздо лучше? ```
Reported by `oasychev` on 2012-01-01 19:28:52
-
reporter
``` Еще один (надеюсь последний пока) небольшой рефакторинг: is_match теперь функция, а не поле результата. Это поможет централизовать редактирование условий, что считать наличием совпадения.
Но матчерам опять требуется восстановить работоспособность. :(
P.S. Изменения вытолкнул, могут содержать небольшие ошибки - тестить временно не на чем. ```
Reported by `oasychev` on 2012-01-03 09:17:29
-
``` NFA восстановлен. Там, кстати, какие-то фейлы в тестах вопроса, касательно is_match... ```
Reported by `vostreltsov` on 2012-01-03 12:29:59
-
reporter
``` Фейлы появились уже после замены на функцию? Тогда надо будет смотреть как доберусь до компа с Денвером...
А чего в тесте от backtracking осталось tqtype_? ```
Reported by `oasychev` on 2012-01-03 14:11:21
-
reporter
``` Мне, кстати, понравились изменения в НКА-матчере в связи с последним восстановлнением: некоторые внутренние проверки вместо is_match (критерии определения которого могут и изменится) стали использовать более стабильные по смыслу параметры (длину совпадения например).
Дмитрий, когда оживет ДКА? ```
Reported by `oasychev` on 2012-01-03 20:02:38
-
reporter
``` В классе результатов матчинга появились константы qtype_preg_matching_results::UNKNOWN_NEXT_CHARACTER вместо пустой строки и qtype_preg_matching_results::UNKNOWN_CHARACTERS_LEFT если неопределено, сколько символов осталось. Облегчает понимание кода и дальнейшее изменение этих значений.
Необходимо поправить матчеры если они там используются. Касается главным образом ситуации отсутствия следующего символа, длина у вас вроде бы определяется всегда... ```
Reported by `oasychev` on 2012-01-09 20:36:19
-
reporter
``` Добавлена константа qtype_preg_matching_results для отсутствия совпадения, использующаяся в массивах и индексов и длин. Используйте в матчерах, чтобы избежать проблем при возможной смене этого значения. Только вместо > лучше использовать != - так надежнее если значение станет, например, большим. ```
Reported by `oasychev` on 2012-01-14 05:01:21
-
reporter
``` В конфиге необходимо обновить актуальное значение! НКА матчер является значением по умолчанию для этого поля, но таблица может хранить любой из матчеров - какой реально выберут на данном сайте.
Дописать апгрейд срочно, я пока такое мержить не могу... ```
Reported by `oasychev` on 2012-01-14 14:36:44
-
reporter
``` Я еще немного "поломал" матчеры в связи с работой над issue 70. Но поправить будет несложно. Обратите внимание - изменился конструктор qtype_preg_matching_results.
Удалено поле next. Вместо него есть поле correctending, где помещается строка корректного завершения (возможно неполного), correctendingcomplete (полное ли завершение, т.е. даст ли оно полное совпадение при матчинге) и correctendingstart - на случай, если исправлять надо не с того места, где остановился матчинг. Кроме того, correctending может быть равна DELETE_TAIL, в случае если нарушен последний $ и проблема не в добавлении символов, а удалении их с конца.
К 2.2 достаточно сделать следующие изменения: 1) вернуть - хотя бы и один символ - в correctending вместо next - просто исправьте вызовы конструкторов qtype_preg_matching_results; 2) вернуть qtype_preg_matching_results::DELETE_TAIL в качестве корректного завершения, если нарушено совпадение последнего(!) ассерта $ 3) вернуть correctendingcomplete истиной если содержимое correctending позволяет довести матчинг до конца (в случае одного символа - если он последний, НО - не путайте с последним переходом автомата, т.к. после него могут стоять ассерты, обратные ссылки с пустым совпадением и т.д.) ```
Reported by `oasychev` on 2012-01-20 12:46:03
-
reporter
``` Валерий, у меня такое впечатление что вы не совсем поняли назначение этих переменных. 1) correctendingcomplete должен выставляться в истину если введенная строка + correctending дают в результате полное совпадение. Грубо говоря, от него зависит вывод многоточия после подсказки - если оно ложно, то многоточие выводится - мол это еще не все. 2) если совпадение полное, такое впечатление что у вас зачем-то выставляется correctendingstart на один символ назад - или что-то подобное, не успел оттесить. Попробуйте в превью вопроса с введенным правильным ответом нажать подсказку. Почему-то съедается последний символ и появляется многоточие... ```
Reported by `oasychev` on 2012-01-25 07:48:47
-
reporter
``` Вытолкнуто очередное изменение формата возвращаемых матчером данных.
На этот раз строка, дающая улучшенное совпадение, возвращается еще одним объектом qtype_preg_matching_results в поле extendedmatch, что дает возможность вернуть не только добавляемые символы,но и подмаски, лексемы и прочее.
Роль corectendingcomplete теперь выполняет extendedmatch->full.
Поле extensionstart заполнять не нужно - оно вычисляется автоматически матчером (через функцию validate после нахождения совпадения). DELETE_TAIL теперь тоже не требуется, все что идет после extensionstart в любом случае считается удаляемым. ```
Reported by `oasychev` on 2012-01-29 20:17:03
-
``` У меня вопрос по полю $str в матчере - зачем оно там нужно? Свойством матчера является регекс, но никак не строка, ей место в matchresults, ИМХО.
В конструкторе вижу вызов set_source_info, хотя никакого source вообще еще нет. Разве не логичнее вызывать эту функцию ТОЛЬКО внутри match() или match_inner()? ```
Reported by `vostreltsov` on 2012-01-31 17:13:30
-
reporter
``` В принципе поле str из матчера можно удалить - в кеше оно не используется. Или я удалю.
Вызов set_source_info в конструкторе нужен для нормальной работы invalidate - там ведь не только строка, а и количество подмасок и подобная информация о регексе. ```
Reported by `oasychev` on 2012-02-01 07:28:14
-
reporter
``` В конструкторе в set_source_info можно передать пустую строку. ```
Reported by `oasychev` on 2012-02-01 07:29:35
-
reporter
``` Я уже устал говорить о том, что использовать поле с результатами матчинга для взаимодействия с обратной ссылкой неправильно.
Лучше иметь отдельные функции для взаимодействия с узлами в процессе матчинга. Можно сделать им специальный префикс или суффикс (for_nodes например) чтобы отделить итоговые результаты от промежуточных. ```
Reported by `oasychev` on 2012-02-01 13:38:48
-
``` Вы говорили, что стоит передавать отдельный объект в preg_leaf::match для этой цели.
Мне кажется, можно не создавать отдельный класс для этого, а реализовать нужные функции в тех же наследниках matching_results. Грубо говоря, не важно какого класса будет этот объект, главное чтобы были нужные методы. Было бы очень удобно. Что скажете? ```
Reported by `vostreltsov` on 2012-02-01 21:00:52
-
reporter
``` Валерий, посмотрите на исключение в test_question.php при генерации подсказки при отсутствии совпадения (тест в test_question.php строка 264 и далее) $matchresults->extendedmatch->index_first[0] равен -1 почему-то (хотя остальные индексы в порядке) ```
Reported by `oasychev` on 2012-02-02 15:07:30
-
reporter
``` Вариант "неважно какого класса объект - лишь бы были методы" - нормален, но нужно оформить эти методы в интерфейс. Так будет место, где виден четкий перечень методов. Любой класс может сказать что он поддерживает интерфейс.
И я бы попросил возвращать "чистые" matching_results а не наследники как итоговый результат совпадения. Иначе отлаживать вопрос неудобно - print_r выдает просто кошмарик... Функцию создающую один объект на основе другого сделать элементарно. ```
Reported by `oasychev` on 2012-02-02 15:18:34
-
reporter
``` Дмитрий, поскольку теперь возвращается строка а не один символ, неплохо бы доработать до генерации не просто след. символа, а полного завершения. Если это несложно (допускается иметь некоторые проблемы в сложных случаях с \b) то это следует сделать срочно к релизу - это открывает возможность к некоторым новым полезным функциям. ```
Reported by `oasychev` on 2012-02-04 19:38:14
-
reporter
``` Введена новая константа способностей матчера, показывающая что сгенерированное им продолжение всегда будет полным совпадением. Я так понимаю НКА нужно уже возвращать ее как TRUE в is_supporting и надеюсь что ДКА тоже скоро к этому присоединится. ```
Reported by `oasychev` on 2012-02-08 11:15:15
-
reporter
``` В основном клоне preg 2.2 вытолкнут код, обновленный до Moodle 2.2
Просьба обновить свои тестировочные серверы до версии Moodle 2.2 ```
Reported by `oasychev` on 2012-02-14 13:25:46
-
reporter
``` Дмитрий, обновите матчер и восстановите работоспособность! В частности что касается использования нового класса строки, узлы уже перешли на нее, а ваш матчер - нет... ```
Reported by `oasychev` on 2012-07-19 15:51:13
-
reporter
Reported by
oasychevon 2012-12-14 17:09:55 - Status changed:Done - Log in to comment
``` Столкнулся с такой проблемой: Fatal error: Class 'qtype_preg_accepting_error' not found in W:\home\moodle.local\www\question\type\preg\preg_regex_handler.php on line 225 ```
Reported by `Xapuyc7` on 2011-12-29 16:16:05