Make the captcha API better

I think 'solution' is a much more sensible variable name than 'value' (which is a crap variable name in this situation IMHO).
Because the new captcha module will be a mayor rewrite, I think we can/should afford breaking the API now. It will be much harder/painful to do it in the future, after the new captcha module will become "official".

a good API is important: check http://buytaert.net/how-to-design-a-good-api (blog of the drupal leader)

I agree that 'value' is a horrible variable name for the backend API. When designing the reCAPTCHA module, this is one of the issues I ran into with it. Seeing how there wasn't any API documentation, I pretty much had to randomly guess until I figured out that 'value' was the answer you want to be returned from hook_captcha's validation. 'solution' makes much more sense, and I think we should consider switching to use it.

Yes, it will break the current API, but to my knowledge, the only external module that uses the new Captcha 2.x API is reCAPTCHA. And I'm in support of the move to 'solution' as opposed to 'value'. Any thoughts?

Another note is that 'captcha_response' was renamed to 'captcha_answer'. So if a Captcha module was calling form_set_error('captcha_response', 'blah') to set their own error message, you'd now have to use form_set_error('captcha_answer', 'blah') instead. captcha_response made more sense to me, personally.

So should we also switch captcha_answer back to captcha_response?

So should we also switch captcha_answer back to captcha_response?

No problem for me.
When I started rewriting the captcha module, it was originally called 'captcha_challenge' however, and I renamed it to 'captcha_answer', which seemed more sensible to me. But 'captcha_response' is even better.

Because I think that having a good API is very important and because the current captcha API is unobvious (see #2, #3, #4), I think we should really provide a better API: solution instead of value for the captcha solution and captcha_response instead of captcha_answer for the response form element.
I understand however that breaking an API has its downsides such as mismatch/compatibility problems.

In this patch I tried to push the API update as painless as possible:
* The API is changed as suggested above.
* But modules that follow the old captcha API will still work: their returned values are translated to the new API and a warning is written to the watchdog log.

I reworked my patch of #6 a bit:
I added an extra check with warning on the captcha settings page, so administrators know in advance which captcha types could give problems because they follow the old/an unknown API.

new version of my patch (see #6 and #7):
* added information about response preprocessing to the API documentation (see http://drupal.org/node/160769)
* minor updates of the API documentation and removed minor bugs

I personally am against the notion of backwards compatibility with the API. Consider reading this article: On backwards compatibility: the drop is always moving. If we keep everything backwards compatible, we'll end up with a mess of code that will be hard to maintain. Keep a clean single API that doesn't change. Choose the best method, implement it, and stick with it. Remember, once the API is chosen, it cannot change...

A good test for it would be to test against reCAPTCHA as I wrote it to work with the beginnings of the Captcha 3.x API.

If we keep everything backwards compatible, we'll end up with a mess of code that will be hard to maintain

I agree and that's why I putted comments like "TODO: delete this stuff when not needed anymore" in the code. I provided this "API compatibility check stuff" for captcha challenge developers mainly.
My patch is not about keeping "backwards compatibility" however, because the API does change considerably. I just added a feature to generate warnings and error messages which should be helpful for developers to port their captcha challenge to the new API.
If the parts with the "TODO: delete this" comments are removed, the captcha module won't be "backwards compatible" anymore.

Keep a clean single API that doesn't change.

I totally agree and I think we should get the API just right before captcha v3 "stable" is released.

To quote myself:

I think we should get the API just right before captcha v3 "stable" is released.

I was thinking about an extension to the captha API. Right now the captcha API only supports validation of one input widget ('captcha_response' aka 'captcha_answer'). I think this is a bit too limited, as I can imagine captcha challenges where the user has to interact with several widgets (e.g. some radio buttons and a text field).

Maybe we should change the 'preprocess' part of the captcha API so a captcha challenge can preprocess the form_values and return one string to validate for example.
I will have a look how to implement this.

any toughts on this

After getting some time and reviewing your #8 patch, I see five different things going on here:

1) Backwards compatibility check

Although it's nice to leave this in until all known captcha modules use the new API, I think we should skip it and put work into helping them migrate to the new API changes instead (write documentation, submit patches, etc). Having a background compatibility check also means more code for us to maintain. I know you mean to remove it afterwards, but it would just be better to get this straight out the door first. If we're changing the API, it's something that WE have to deal with.

2) Documentation fixes of @todo elements

You really should be using the issue queue manager on drupal.org. It would be best to remove them from the code and make an issue for each one instead.

3) Rename from $captcha['value'] to $captcha['solution']

This is a good move because "solution" makes more sense than "value".

4) Rename 'captcha_answer' to 'captcha_response'

"Response" or "answer" are fine, but if we are changing the API, I'd rather use "response".

5) Documentation fixes in captcha_api.txt

These documenation changes are fine with me.

I'm in agreement with item 3, 4 and 5, but have some issues with 1 and 2. I'm up for renaming $captcha['value'] to $captcha['solution'] and the answer to response. The documentation change in captcha_api.txt will help a lot, and we'll have to document the changes we make. Another thing we should do is remove the @todos and put up issues for each one that still isn't yet resolved.

at #12:

Dropping (1) is totaly fine for me. I just added this to make the API changes more likely to happen. I really want the API to change to get a better one (like you suggested: On backwards compatibility: the drop is always moving), but I felt that wundo is not that enthusiastic about the API change (I could be wrong, I hope so).

About removing the @todo items in the code (2): no problem. I used this todo list during my rewrite before it was accepted to the CVS repository. Now that it's in CVS it is indeed better to use the tracker.

In attachment the patch reworked, with just a minor relic from (1): 5 lines with a compatibility check to ease the API change for challenge developers (an issue should be opened to remove this when not needed). The API translation from old to new is dropped. All @todo's are removed too, I don't think one of them needs an issue in the tracker anymore.

Small variation on the patch from #13: now also the small compatibility check is dropped.

This patch changes the preprocess hook of the captcha API a bit to add support for multi widget captcha challenges.
The preprocess hook now gets passed all submitted form values, so it can pick the ones he needs.
I added documention about this in captcha_api.txt

I also changed the tripple quotes to < ?php ... ? > .

From now on, could you create new issues other then re-opening this one? "Multi widget captcha challenges" would work ;-) .

Although I tested this with reCAPTCHA (it uses preprocess) and seems to work with it, I'm still not entirely sure why it's required. Could you provide a test case?

From now on, could you create new issues other then re-opening this one? "Multi widget captcha challenges" would work ;-) .

Indeed: http://drupal.org/node/169132