A "Login - Register" link would be very cool

I'm trying to work my way around this as you can see from here. I will try to do some work to offer some suggestions for the links handling in Discuss This, it doesn't look too complicated (I'm frightened to see how complicated Drupal seems to have gotten since I last tried hacking a module!).

I thought I would start off simple just using the things that definitely do work, ie the Drupal login and register forms. So I added this fairly self-contained code at the bottom of the discussthis_link function:

        // Special case if the user is anonymous and does not have access to topics
        // then display the Login links
        if ($user->uid == 0 && !user_access('initiate discuss this topics')) {
            if ($topic_nid == 0) {
            // The topic does not exist
                $destination = 'destination=discussthis/new/'. $node->nid;
                $appeal = t('Discuss This!');
            }
            if ($topic_nid) {
                $destination = 'destination=node/'. $topic_nid;
                $appeal = t('participate in the discussion');
            }
            if (variable_get('user_register', 1)) {
            // Users can register themselves.
                $href = t('<a href="@login">Login</a> or <a href="@register">register</a> to ' . $appeal, array('@login' => url('user/login', array('query' => $destination)), '@register' => url('user/register', array('query' => $destination))));
            }
            else {
            // Only admins can add new users, no public registration.
                $href = t('<a href="@login">Login</a> to ' . $appeal, array('@login' => url('user/login', array('query' => $destination))));
            }
            $links['discussthis'] = array(
                'title' => $href,
                'html' => TRUE,
            );
        }

It works! At least it seems to work for my use case, though there might be some subtleties that need adding to take care of anonymous comment users etc.

I've looked at this a bit more closely and actually it is possible to set the user registration (for when users are allowed to register themselves) so that either they have to confirm their registration with an e-mail - in which case you are right about that - or they don't (ie they can register, choose a password, and are directly logged in). This code takes care of both cases:

        // Special case if the user is anonymous and does not have access to topics
        // then display the Login links
        if ($user->uid == 0 && !user_access('initiate discuss this topics')) {
            if ($topic_nid == 0) {
            // The topic does not exist
                $destination = 'destination=discussthis/new/'. $node->nid;
                $appeal = t('Discuss This!');
            }
            if ($topic_nid) {
                $destination = 'destination=node/'. $topic_nid;
                $appeal = t('participate in the discussion');
            }
            if (variable_get('user_register', 1)) {
            // Users can register themselves without e-mail verification
                if (!variable_get('user_email_verification', 1)) {
                    $href = t('<a href="@login">Login</a> or <a href="@register">register</a> to ' . $appeal, array('@login' => url('user/login', array('query' => $destination)), '@register' => url('user/register', array('query' => $destination))));
                }
                else {
            // Users can register themselves but have to go through e-mail verification process
                    $appeal = $appeal . t(' (you will have to return to the discussion after validating your new password by e-mail)');
                    $href = t('<a href="@login">Login</a> or <a href="@register">register</a> to ' . $appeal, array('@login' => url('user/login', array('query' => $destination)), '@register' => url('user/register')));
                }
            }
            else {
            // Only admins can add new users, no public registration.
                $href = t('<a href="@login">Login</a> to ' . $appeal, array('@login' => url('user/login', array('query' => $destination))));
            }
            $links['discussthis'] = array(
                'title' => $href,
                'html' => TRUE,
            );
        }

I've tried it and it works (I think)

Another question - a pointer really. I've been trying to work out how you could get this to work with Ajax Login/Registry and I'm really perplexed to discover how you actually go about finding out what are the attributes of a form so that you can play with them. I can see that you do this (for example with forum_node_form), but how do you discover what are the fields in forum_node_form?

Hmmmm... Perhaps displaying the "Registry" link could be made a configurable option? That would handle the simple cases. And I suppose if you wanted to you could even have the URL to the login form as a configurable option as well, that way you could handle different login cases?
Or you could just keep it simple and handle vanilla login/registry (like I have done) with a configurable option to use display or not the login/registry links. I know that to have these links is important for me, since I want to make it as easy as possible for people to join the forum, but I'm sure you're right that there are lots of other potential use cases which it would be too complicated to treat.

The last time I tried to create a patch nobody could get it to work, but I can try again... I am working on an upgrade this weekend which (all being well) will use Discuss This! I will try to create a patch if I can get this to work ok, and post it here.

I've been thinking about this some more and I'm not so sure that there should be separate configuration options. I'm inclined to think that this should be handled through permissions, for example:
1) Overall access to the "Login or register" links can be managed simply by correcting the permissions that I have already used in the proposed patch, as so:

if ($user->uid == 0 && !user_access('initiate discuss this topics') && user_access('access discuss this links')) {
...
}

So if you are not logged in and the anonymous user has not got "access discuss this links" permission, then the links will not show.

2) To handle the use case of the access to "Quick register", I suggest solving this by adding a permission called "quick register if anonymous". Then all you have to do is to alter the access to the "login or register" links as so:

if (variable_get('user_register', 1) && user_access('quick register if anonymous')) {
...
}

What do you think? I like this solution because it is more economical with the code and reduces the number of configuration options. I've tried it out and it seems to work.

In that case I won't try to do a patch, especially since I just discovered that there's another improvement just been added...