Marking on "words" not strings within words

rbrahmson

Am I wrong in that the "Discussion Marker" plugin marks the discussion even if the term is within another word? If so it is pretty unusable for us.

Is there a way to make it look for words (which of course could be delimited by spaces,commas, etc.).?

hgtonight

According to the source here: https://github.com/bleistivt/DiscussionMarker/blob/27544291eaef7062ada00ce2942a22f79520ae3b/class.discussionmarker.plugin.php#L67

The regex uses the word boundry character \b on the beginning and end so it should not match in words withing words.

Bleistivt

@hgtonight said:
According to the source here: https://github.com/bleistivt/DiscussionMarker/blob/27544291eaef7062ada00ce2942a22f79520ae3b/class.discussionmarker.plugin.php#L67

The regex uses the word boundry character \b on the beginning and end so it should not match in words withing words.

Yes, it does

...in the new version I just uploaded.

@rbrahmson You can try out if the new version works for you

hgtonight

@Bleistivt said:
rbrahmson You can try out if the new version works for you

Should have checked that commit time.

rbrahmson

To @hgtonight and @Bleistivt: Thank you -- this new version corrects the issue.

rbrahmson

So a bit of confession -- I am not a programmer, not even an aspiring one, and I wanted the discussion marker plugin to also search the body of the discussion, not just the title. I decided to take a look at the 1.7 source and try something out- sort of a gamble. If it worked I figured I'd put it on this discussion as a suggestion. Well, the gamble worked for a very short while - until I discovered the substring issues which were fixed in Ver 1.8 but broke my code (if you can even call my amateurish changes code).

Here is what I did on Version 1.7 (changed marked with //PA):

    protected function DisplayMarker($Sender) {
            $Discussion = $Sender->EventArguments['Discussion']; //Original source
            $title = val('Name', $Discussion);                                    //Original source
        $thebody = val('Body', $Discussion);                               //PA body contents

and later on

$pos = stripos($title, $Marker);                     //Original source 
$posbody = stripos($thebody, $Marker);       //PA Search for body contents as well

and the next line changed to:
if (is_numeric($pos) || is_numeric($posbody)) {

These are all my changes to 1.7. I didn't expect it to work but it did. But now, these won't work any more because Version 1.8 changed the function...

Can someone implement this feature (possibly as an admin set option - performance was fine for my small forum) on the new Ver 1.8 (and I'm sure in a more professional way than I attempted).

Bleistivt

I "inherited" this plugin from peregrine and had not changed anything until now, so when I applied the fix I also adapted it to the new coding standard and shortened the function a little.

With your extended functionality (wich doesn't look amateurish at all), it would look like that from line 63:

    $title = val('Name', $args['Discussion']);
    $body = val('Body', $args['Discussion']);

    $markerArray = preg_split('/ *, */', C('Plugins.DiscussionMarker.WordList'));
    foreach ($markerArray as $marker) {
        if (!preg_match('/\b'.$marker.'\b/', $title.' '.$body)) {
            continue;
        }

rbrahmson

Thanks @Bleistivt , before I test it just a thought - instead of searching twice in both the body and the title, would it be more efficient to concatenate the two (with a period in between to prevent inadvertent string matching) and perform one search on the concatenation?

(I do remember something from a PL/1 coding class thirty years ago;-)

Bleistivt

I don't know, probably. I wrote it like that, because it's shorter and that is all that counts.

Minimal performance improvements are neglible if you can achieve cleaner code that is easier to maintain.
Most of the plugins execution time is probably spent dispatching to the event handlers or some other PHP magic, anyway.

Note that it loops over all the markers, so the number of markers you have has a direct influence on the speed.

I did a small test with 1000 markers. I didn't notice a speed difference.

rbrahmson

I want to thank @hgtonight and @Bleistivt for helping in directing, correcting, and updating this plugin. I took the most recent version 1.8.1, added @Bleistivt suggestion to modify the plugin to search the body of the discussion as well, and then I made a change to the options screen adding that body search as an admin option. Thus the update may be useful to all. Here are the changes:

To discussionmarket-settings.php:

        echo $this->Form->CheckBox('Plugins.DiscussionMarker.GroupLabels', "");
        echo "</td>";
        echo "</tr>";
        echo "<td>";
        echo T("Search discussion body as well (otherwise only title is searched)");
        echo "</td>";
        echo "<td>";
        echo $this->Form->CheckBox('Plugins.DiscussionMarker.SearchBody', "");
        echo "</td>";
        echo "</tr>";

To the class.discussionmarker.plugin.php:

        protected function displayMarker($sender, $args) {
        $title = val('Name', $args['Discussion']);
        $body = val('Body', $args['Discussion']); //PA 
        $fulltext = $title.' '.$body;             //PA

        if (c('Plugins.DiscussionMarker.SearchBody', false)) {

           $fulltext = $title.' '.$body;      //PA
        } else {
           $fulltext = $title;                //PA  
        }

        $markerArray = preg_split('/ *, */', c('Plugins.DiscussionMarker.WordList'));

        foreach ($markerArray as $marker) {
            if (!preg_match('/\b'.preg_quote($marker, '/').'\b/', $fulltext)) {
                continue;

Hope this will help someone and perhaps the plugin author (Peregrine?) may find it useful to add to the next version.

businessdad

@Bleistivt said:
Yes, it does
...in the new version I just uploaded.

A clbuttic mistake.