Failure checking improvement

[Neil Hodgson]

Scintilla has a mechanism for reporting failures: SCI_GETSTATUS can be called to return the current status. This is either SC_STATUS_OK or another SC_STATUS_* value to report a problem like SC_STATUS_BADALLOC for memory exhaustion.

This is a bit clumsy as it takes an extra call. Many applications do not currently check that their calls to Scintilla succeed. In turn, this leads to Scintilla code rarely checking for failures and setting the status. Ideally, a bad status would throw an exception in the language of the calling application.

There could be an extended version of the ‘direct function’ that returns (through a pointer argument) the current status value and avoids the second call. Something like:

// In Scintilla.h
typedef sptr_t (*SciFnDirectStatus)(sptr_t ptr, unsigned int iMessage, uptr_t wParam, sptr_t lParam, sptr_t *pStatus);
#define SCI_GETDIRECTFUNCTIONSTATUS 6789

// In application code, setup
#include "Scintilla.h"
SciFnDirectStatus pSciMsgStatus = (SciFnDirectStatus)SendMessage(hSciWnd, SCI_GETDIRECTFUNCTIONSTATUS, 0, 0);
sptr_t pSciWndData = (sptr_t)SendMessage(hSciWnd, SCI_GETDIRECTPOINTER, 0, 0);
struct ScintillaFailure { sptr_t status; ScintillaFailure(sptr_t status_) : status(status_) {} };

// now a wrapper to call Scintilla directly
sptr_t CallScintilla(unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
sptr_t sciStatus = SC_STATUS_OK;
sptr_t ret = pSciMsgStatus(pSciWndData, iMessage, wParam, lParam, &sciStatus);
if (sciStatus != SC_STATUS_OK) {
throw ScintillaFailure(sciStatus);
}
return ret;
}

Neil

[Teodor Petrov]

On 12/13/2016 02:22 AM, Neil Hodgson wrote:
> if (sciStatus != SC_STATUS_OK) {
> throw ScintillaFailure(sciStatus);
> }
>

I hope you don't intend to make this mandatory.
This will make our c++ code really ugly. We'll have to wrap every call
of Scintilla in try { ...} catch (...) {} statements.
What about using an assert instead or adding a function parameter that
decides what to happen with the error?

/Teodor

[Mike Lischke]

> On 12/13/2016 02:22 AM, Neil Hodgson wrote:
>> if (sciStatus != SC_STATUS_OK) {
>> throw ScintillaFailure(sciStatus);
>> }
>>
> I hope you don't intend to make this mandatory.

I got it as this is a suggestion only. You can of course have our own status handling, without throwing an exception.

> This will make our c++ code really ugly. We'll have to wrap every call of Scintilla in try { ...} catch (...) {} statements.
> What about using an assert instead or adding a function parameter that decides what to happen with the error?

Mike
--
www.soft-gems.net

[Lex Trotman]

On 13 December 2016 at 18:18, 'Mike Lischke' via scintilla-interest

<scintilla...@googlegroups.com> wrote:
>
>> On 12/13/2016 02:22 AM, Neil Hodgson wrote:
>>> if (sciStatus != SC_STATUS_OK) {
>>> throw ScintillaFailure(sciStatus);
>>> }
>>>
>> I hope you don't intend to make this mandatory.
>
> I got it as this is a suggestion only. You can of course have our own status handling, without throwing an exception.

I am sure Neil understands that Scintilla must not allow exceptions to
escape the C API or it would be unusable from C.

>
>> This will make our c++ code really ugly. We'll have to wrap every call of Scintilla in try { ...} catch (...) {} statements.
>> What about using an assert instead or adding a function parameter that decides what to happen with the error?
>
>
> Mike
> --
> www.soft-gems.net
>

> --
> You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-inter...@googlegroups.com.
> To post to this group, send email to scintilla...@googlegroups.com.
> Visit this group at https://groups.google.com/group/scintilla-interest.
> For more options, visit https://groups.google.com/d/optout.

[Neil Hodgson]

Teodor Petrov:

> I hope you don't intend to make this mandatory.
> This will make our c++ code really ugly. We'll have to wrap every call of Scintilla in try { ...} catch (...) {} statements.
> What about using an assert instead or adding a function parameter that decides what to happen with the error?

These 2 lines are what may be added to the Scintilla interface:

typedef sptr_t (*SciFnDirectStatus)(sptr_t ptr, unsigned int iMessage, uptr_t wParam, sptr_t lParam, sptr_t *pStatus);
#define SCI_GETDIRECTFUNCTIONSTATUS 6789

Whether this is used and how it is used is a choice for the application or wrapping library.

For C++, the expected mechanism for errors is to use exceptions so that was what was illustrated. For other languages you might use an optional type or return the status code.

Neil

[Teodor Petrov]

On 12/14/2016 12:18 AM, Neil Hodgson wrote:
>
> These 2 lines are what may be added to the Scintilla interface:

Good, sorry for misunderstanding your original post.

/Teodor

[Matthew Brush]

On 2016-12-13 02:18 PM, Neil Hodgson wrote:
> Teodor Petrov:
>
>> I hope you don't intend to make this mandatory.
>> This will make our c++ code really ugly. We'll have to wrap every call of Scintilla in try { ...} catch (...) {} statements.
>> What about using an assert instead or adding a function parameter that decides what to happen with the error?
>
> These 2 lines are what may be added to the Scintilla interface:
>
> typedef sptr_t (*SciFnDirectStatus)(sptr_t ptr, unsigned int iMessage, uptr_t wParam, sptr_t lParam, sptr_t *pStatus);
> #define SCI_GETDIRECTFUNCTIONSTATUS 6789
>

Hi Neil,

Would it be possible to also include a strerror()-like facility with
this too? Otherwise each app would have to maintain their own
code->message map if they wanted to provide details to users/callers.
This would make it easier to wrap in a typical C++ stdlib exception or a
GError or such, for example:

::sptr_t ec(0);
SSM(x, m, w, l, &e);
throw std::runtime_error(reinterpret_cast<const char*>(
SSM(x, SCI_GETSTRERROR, ec, 0)));

or:

sptr_t ec=0;
SSM(x, m, w, l, &e);
g_set_error(err, err_quark, ec, "scintilla error: %s",
(const char*) SSM(x, SCI_GETSTRERROR, ec, 0));

Regards,
Matthew Brush

[Neil Hodgson]

Matthew Brush:

> Would it be possible to also include a strerror()-like facility with this too? Otherwise each app would have to maintain their own code->message map if they wanted to provide details to users/callers.

I expected this to be of interest to developers rather than users with log messages like:
Scintilla failure: status 1001 for method 2150

There could be a mapping to error strings if someone wants to implement it but that is orthogonal to this proposal - it could be implemented now even if SCI_GETDIRECTFUNCTIONSTATUS isn’t.

Neil