Kroc / NoNonsenseForum

A free, open source, PHP-based simple discussion forum. It favours removing barriers to conversation rather than massaging egos. Download Here: https://github.com/Kroc/NoNonsenseForum/archive/master.zip
http://camendesign.com/nononsense_forum
Other
247 stars 34 forks source link

Give Titles IDs and link to themselves #157

Closed Kroc closed 11 years ago

Kroc commented 11 years ago

Titles (`:: title) should have an HTML ID, generated using the transliteration method (safeTransliterate), and contain hyperlinks to themselves so that users can easily share links direct to post headings.

For consideration: The URL generated needs to be unique on the page, not only just within the post; should NNF make any effort to ensure a unique URL, or is this just not likely enough to be a real problem?

Kroc commented 11 years ago

Fixed in development branch, but does not yet check for uniqueness.

Zegnat commented 11 years ago

Instead of checking the whole thread for uniqueness, is it possible to just prepend it with the post’s unique ID? This way it only needs to check for uniqueness within the one post — which should be more easy as the parser just needs to remember all the safeTransliterate()s for this one loop.

E.g. the ‘Easy games’ header in reply #7 on ‘Project: Playing card rules on a pack of cards!’ should become:

<h2 id="60kcys8ga3lp::easy_games">
    <a href="#60kcys8ga3lp::easy_games">:: Easy games</a>
</h2>

It also makes for nice hierarchical URLs, if you subtract the ::easy_games from the link it will still take you to the post that contained the header. (Assuming that the finalised HTML will include an absolute URL including the page number +1.)

I don’t see any references to the current post’s ID in the formatText-function, so this might not be possible without accessing $rss either way. But still thought I should throw it in here.

Kroc commented 11 years ago

Damn, that's a really nice idea, thanks!

Edit: $RSS is not available at new-thread creation, however with both new-thread and replies, the ID is known beforehand and thus could be passed into formatText via a parameter.

Zegnat commented 11 years ago

with both new-thread and replies, the ID is known beforehand and thus could be passed into formatText via a parameter.

That pretty much solves the uniqueness checking then. I would implement it myself, but I am still a little intimidated by your formatText and didn’t know enough about the whole ID management. Hope to see this land.

Do you have any plans as to when the next stable lands? Max number of changes per update so far has been 9, the current development branch (v23) stands at 8.

Kroc commented 11 years ago

I'm happy to do the change myself, it's an easy one. Next stable version will be before the new year :)

Kroc commented 11 years ago

Inter-post uniqueness fixed in dev. Just need to check titles within the post, shouldn't be much dissimilar to the check for unique filenames.

Zegnat commented 11 years ago

I bastardised the loop for file-names and ended up with this (if I ever figure out the pull request thing I promise I will use it):

    /* titles
       -------------------------------------------------------------------------------------------------------------- */
    //example: :: title
    $allHeaders = array();
    $replace = ''; while (preg_match (
        '/(?:\n|\A)(::.*)(?:\n?$|\Z)/mu',
        //capture the starting point of the match, so that `$m[x][0]` is the text and $m[x][1] is the offset
        $text, $m, PREG_OFFSET_CAPTURE,
        //use an offset to search from so we don’t get stuck in an infinite loop
        //(this isn’t valid the first time around obviously so gives 0)
        @($m[0][1] + strlen ($replace))
    )) {
        //the ID for the current title
        $translit = safeTransliterate (strip_tags ($m[1][0]));
        //if a title already exsits with that ID, append a number until an available ID is found.
        $c = 0; do $titleID = $translit.($c++ ? '_'.($c-1) : '');
        while (in_array ($titleID, $allHeaders));
        //add the current ID to the list of used IDs
        array_push ($allHeaders, $titleID);
        $text = substr_replace ($text, $replace =
            //create the replacement HTML, including an anchor link
            //(note: inline code spans in titles don't transliterate since they've been replaced with placeholders)
            "\n\n<h2 id=\"$post_id::".$titleID.'">'.
                "<a href=\"#$post_id::".$titleID.'">'.$m[1][0].'</a>'.
            "</h2>\n",
            //where to substitute
            $m[0][1], strlen ($m[0][0])
        );
    }

It works for me. Except I totally forgot about appends. This cannot check for uniqueness within appends because when we append more text the parser does not go through the whole post anew. I’m guessing $rss will still be needed.

Kroc commented 11 years ago

I hadn't thought about appends, but I have a good idea for that: we can simply use strip_tags on the original text -- turning it from HTML back to Markup -- add the appended text, and then format again; then the appended headings will be as natural as the originals. Isn't it nice to have a sane markup syntax?

Kroc commented 11 years ago

Done and dusted :) There couldn't be a simpler, more elegant solution.

NOTE: We cannot check appended titles for uniqueness within the whole post until we first fix #146 (move append divider to markup), since we need to reverse the existing post HTML into text, add the divider, add the append, and then process the whole.