need help with testing of glossary patch

need help with testing of glossary patch

by Petr Skoda -
Number of replies: 4
Picture of Core developers Picture of Documentation writers Picture of Peer reviewers Picture of Plugin developers
I reported several bugs in glossary but it seems nobody wants to fix them, so I tried to make the patch myself.

The fix solves following bugs:
  1. #1086
  2. #1145
  3. #1295
  4. #1366
  5. #1232 (not sure)
I have completely rewritten the "nolinking" mechanism, so it needs lots of testing and some code review - this is my second attempt to code in php wink

The patch is compatible with the latest CVS version.

skodak
Average of ratings: Useful (1)
In reply to Petr Skoda

Re: need help with testing of glossary patch

by Eloy Lafuente (stronk7) -
Picture of Core developers Picture of Documentation writers Picture of Moodle HQ Picture of Peer reviewers Picture of Plugin developers Picture of Testers
Hi Petr,

I've been taking a look to your code (after some problems with my "diff" because it told me that everything was changed). Tip: run cvs diff -b instead.

I've seen your changes in lib.php and they seem to have sense:

-In glossary_print_entry_concept() you have included the concept inside the <NOLINK> tags to avoid the it become linked to itsef.

-In glossary_print_entry_definition() you have separate all the definition in parts (some of them "tag parts" and the rest "text tags"). Then you have processed all the "text tags" to put the concept and every alias inside <NOLINK> tags to avoid the it become linked to itsef too.

Bugs 1145, 1295 and 1336 should be solved with this...

In view.php you have commentd on
Bug 1086 I suppose

If I've missed something, please, let me know it!!

The code is a bit "sequential" but if it works..tomorrow I'll do more tests and, if nobody has anything against it, I'll start modifying that code in small pieces...

After having changes above, I'll start looking your changes in formats...

Any comment?

Ciao smile
Average of ratings: Useful (1)
In reply to Eloy Lafuente (stronk7)

Re: need help with testing of glossary patch

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Peer reviewers Picture of Plugin developers
Hello.

The <NOLINK> part is exactly as you said - file lib.php

In view.php (bug 1086) I only removed one echo "<br />" (line 445) - actually I moved it to formats/2.php (line 11) and formats/3.php (line 11). The rest of the formats seems better without this line break.

The modification of formats/1.php (some more NOLINKing) may not be necessary - I do not understand this part much.

skodak
In reply to Eloy Lafuente (stronk7)

Re: need help with testing of glossary patch

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Peer reviewers Picture of Plugin developers
There appeared some new issues mainly with wiki and plain text format in glossaries.

Maybe we should close mentioned bugs now and file a new bug for /lib/weblib.php so that it respects (or discards) nolink tags for wiki and plain text formats (we could discuss it with Martin here too).

Or we can add some "ifs" to glossary_print_entry_concept() and glossary_print_entry_definition() and do no "nolinking" for wiki and plain text - IMHO easier solution because there are fewer modifications to 1.3beta and people use mainly HTML format anyway.

If agreed I will wait for CVS update and then will close the bugs.

skodak
In reply to Petr Skoda

Re: need help with testing of glossary patch

by Petr Skoda -
Picture of Core developers Picture of Documentation writers Picture of Peer reviewers Picture of Plugin developers
After some 15 minutes of sleep got to wake up again...

1/ plain text should not be nolinked in mod/glossary/lib.php - because it is not valid HTML (html entities are encoded later in format_text)

solution:
function glossary_print_entry_concept($entry) {
$options->para = false;
if($entry->format==2) {
$text = format_text($entry->concept, $entry->format, $options);
} else {
$text = format_text('<nolink>' . $entry->concept . '</nolink>', $entry->format, $options);
}
if (!empty($entry->highlight)) {
$text = highlight($entry->highlight, $text);
}
echo $text;
}

and

function glossary_print_entry_definition($entry) {
$definition = $entry->definition;
if($entry->format!=2) {
//do nolinking ....

2/ wiki should respect nolink tags, do not modify format_text in weblib.php -> modify wiki_to_html (or better something in lib/wiki.php)

I know nothing about wiki format myself, could somebody please send me any link with some wiki format info? sad