NO_ERROR in websocket.h

[tpatja]

First of all, thanks for an awesome library.

Here's a small annoyance: Currently websocket.h uses NO_ERROR as the first item in the Websocket::Error enum. This causes problems when building on windows as winerror.h does a #define NO_ERROR 0L. It fails to build unless the user is very careful not to include any files that end up including any windows headers before including <WebSocket>. I suggest you rename the enum item.

Thanks.

[vinipsmaker]

First of all, thanks for an awesome library.

You're welcome.

Here's a small annoyance: Currently websocket.h uses NO_ERROR as the first item in the Websocket::Error enum. This causes problems > when building on windows as winerror.h does a #define NO_ERROR 0L. It fails to build unless the user is very careful not to include any files that end up including any windows headers before including . I suggest you rename the enum item.

This is terrible. I can't fix this error without break the API. =O

Here is my proposal to fix this error:

enum Error:
{
#ifndef NO_ERROR
    NO_ERROR,
#endif
    NO_ERROR_WEBSOCKET = 0,
    // ...
};

What do you think about this approach?

And I don't like the NO_ERROR_WEBSOCKET name. If you can think of a better name, please tell me. =P

I can release a new version containing this fix during the weekend.

[tpatja]

I understand renaming can cause problems for current code using the API.

How about putting this in the beginning of the file:

#if defined(NO_ERROR)
# define __WINERROR_WORKAROUND NO_ERROR
# undef NO_ERROR
#endif

..and this in the end:

#if defined(__WINERROR_WORKAROUND)
# define NO_ERROR __WINERROR_WORKAROUND
#endif

This way the API stays the same and all users can #include the file wherever.

[vinipsmaker]

@tpatja, your suggestion is better. Windows users could test if there is a error or not like this:

if (ws.error())
    // ...

and no new name would be required.

I want to port the tests from Tufão 1.0 to Tufão 0.x, but I can't do this right now. I'll do it during the weekend and put this fix in the same release.

Tufão doesn't have a windows maintainer right now. Thanks for the report. =)

[tpatja]

In case you are indirectly asking if I want to be a windows maintainer, I will unfortunately have to decline due to limited time :)

But I am using Tufao in a multiplatform environment (OSX + Windows) and will definitely report (and help fix if time permits) any issues found. So far, it has been smooth sailing, but I'm on QT 4.8 (so 0.x branch of Tufao) and have a hunch that SSL might cause issues once I get around to adding HTTPS and WSS support. (With QT 4.x & windows using SSL means you have to ignore NoError in sslErrors() signal :/)

Regarding this issue, I just send you pull requests.

[vinipsmaker]

@tpatja,

In case you are indirectly asking if I want to be a windows maintainer, I will unfortunately have to decline due to limited time :)

Understood.

But I am using Tufao in a multiplatform environment (OSX + Windows) and will definitely report (and help fix if time permits) any issues found. So far, it has been smooth sailing, but I'm on QT 4.8 (so 0.x branch of Tufao) and have a hunch that SSL might cause issues once I get around to adding HTTPS and WSS support. (With QT 4.x & windows using SSL means you have to ignore NoError in sslErrors() signal :/)

Thanks. =)

Regarding this issue, I just send you pull requests.

I made a comment on one of them. I'll have more time to review it later.

[vinipsmaker]

@tpatja: Thanks for the patch, I'll close the issue once I put a portability note on the documentation.

I'll release a new version (with other news) at the weekend.

[tpatja]

Ok, glad I could be of help.

About portability, it's a good thing windows users of the API will never have to set the Error enum value (they'd have to do Tufao::WebSocket::Error e = (Tufao::WebSocket::Error)0, or some other ugly thing). For checking, a simple if statement is enough, as you said before. Also, to clarify, "plain" QT applications that don't #include windows.h being built on windows are not affected at all.

[vinipsmaker]

[...] I'll close the issue once I put a portability note on the documentation.

Documentation added to master and 0.x branches.