Re: [scintilla] Generating Scintilla wrapper using Scintilla.iface

[Neil Hodgson]

Denis V. Shelomovskij:

> I'm creating a Scintilla wrapper for D programming language. I want this wrapper to be auto-generated using Scintilla.iface but I faced some problems and have to change the file. My repo:
> https://github.com/denis-sh/Scintilla.iface-improvements
>
> I hope changes in `master` branch are obvious and needed for general wrapper generation and at least some of them can be merged in the main repo.

Scintilla.iface is used by downstream projects to determine how they see the API of Scintilla. Changes to this file will have large impacts on existing code and will need strong justification. In particular, adding new types to Scintilla.iface and modifying existing APIs to use these types will break existing use.

API stability is a very important asset of the project allowing users to invest in Scintilla. I will not accept changes that devalue that investment.

> The main problem is that 3 functions are inconsistent (nothing is tested yet, it's just a result of reading Scintilla sources but all other functions looks consistent). You can see them in `strange-funcs` branch (e.g. click Branches->Compare in github to see diff with `master`).

The reasons for some of the older APIs being inconsistent are historical. Initially Scintilla partially emulated the windows text edit and rich edit controls and some of the EM_* messages supported by that control. The old messages are listed in SciMessageFromEM. The text edit control behaved inconsistently and IIRC changed between versions. Scintilla evolved behaviour that worked with various clients that were built with different assumptions about API behaviour. It was much work to get these APIs working as well as they do so don't break that for consistency.

That is: leave all messages mentioned in SciMessageFromEM alone. If there is significant benefit then add a new improved API.

Neil

[Neil Hodgson]

Denis V. Shelomovskij:

At least these changes are just fixes for current *.iface file: https://github.com/denis-sh/Scintilla.iface-improvements/commits/fixes-only

Can they be merged?

   They would have to be reviewed.

   I had a quick look and there is a problem in c37025f with the use of set for APIs like SetSelFore. Properties may be in 2 forms: either simple values or indexed properties where there is one value per valid index. The "get" and "set" feature types should only be used for properties. SetSelFore has two parameters but the first is not an index, instead being a flag specifying whether to use the second parameter. Since it can't be implemented as a property, it shouldn't change to "set".

Really sorry, but I can't imagine how existing *.iface file can be used for automated generation of anything.

   Its been in use in many projects for a long period, so seems to have some value.

   Neil

[Philippe Lhoste]

On 16/06/2012 08:47, Denis V. Shelomovskij wrote:
> Really sorry, but I can't imagine how existing *.iface file can be used for automated

> generation of anything. It just hasn't enough information about function arguments/results
> forcing user to work with strings as pointers and carefully read documentation. Not sure
> if I managed to find appropriate description for every string function exact argument
> type. This situation is too error-prone to leave it as is.

It is designed for automated generation from the start, since it is easier to parse than a
full C file.

It usage starts by generating source files for Scintilla itself, and it is also used by
the Lua interface for SciTE.
I think it is also used by several scripting languages wanting to include Scintilla in a GUI.

--
Philippe Lhoste
-- (near) Paris -- France
-- http://Phi.Lho.free.fr
-- -- -- -- -- -- -- -- -- -- -- -- -- --

[Neil Hodgson]

   After examining the list of proposed API changes at https://github.com/denis-sh/Scintilla.iface-improvements/commits/fixes-only , there are some that appear reasonable and others that don't. Attached are two patches: APIUp.patch contains changes that appear OK and NoSimpl.patch contains the changes I don't want to include.

   The main reason for not including changes is that they try to convert functions with two parameters to properties where the first parameter is not an index. Some other functions are unsuitable as they are irregular such as GetCurLine which returns the caret position, not the size of the string returned.

   Finally, indexed parameters are normally used for fixed sets of indices like a set of styles, not for a dynamic set of indices like the set of lines in a document. This is more of a judgement call but there isn't a great loss of function in requiring access to these as functions instead of properties. Not everything that "get"s data needs to be a property.

   Downstream projects should be aware that changing functions to properties will change bindings generated from Scintilla.iface and this may require code that uses those bindings to be changed,

   Neil

[Greg]

When I started to use Scintilla I wrote myself a script that translated the .iface file into methods in a C++ class, and this worked well for a long time. However, I found the need to have two classes of call: ones that can throw (basically calls that allocate memory) and calls that do not. From that point I worked by looking at a diff of the interface each time a new version was released and handling the changes manually, which is not that big a task as the changes are generally not large. It would be really nice to be able to return to auto-generating the interface... I know that I could make all the calls use the throw detection route, but it feels an unnecessary overhead in many cases. Would you consider adding a "nothrow" to the end of fun, get and set lines that do not throw (or "canthrow" to those that can)? It coud be hidden in a comment before the function definition...

This would likely break existing import scripts, so I quite understand a resounding "No". Perhaps you could poll to see how many projects rely on the .iface file?

[Neil Hodgson]

Greg:

> However, I found the need to have two classes of call: ones that can throw (basically calls that allocate memory) and calls that do not.

It can be quite difficult to work out which calls can throw. Anything that could produce an event, including a macro-record event such as moving the cursor could lead to reentrance and setting the error code. Gradually, std::string and std::vector are infiltrating the code and wherever they are used, there's a possibly of exhausting memory.

> I know that I could make all the calls use the throw detection route, but it feels an unnecessary overhead in many cases. Would you consider adding a "nothrow" to the end of fun, get and set lines that do not throw (or "canthrow" to those that can)? It coud be hidden in a comment before the function definition…

There are multiple ways to add the attribute if it really is warranted. If a method is marked nothrow then changing this could be considered breaking compatibility if clients are relying on it.

Is removing error detection from some methods a significant improvement?

Neil

[Lex Trotman]

Hi Neil,

Its unlikely to be significant.

Although not a disinterested evaluation :) see
http://www.open-std.org/jtc1/sc22/wg21/docs/TR18015.pdf especially the
exceptions part.

Basically it depends on your compiler so much that for a portable app
like Scintilla you can't tell what the effect might be.

Cheers
Lex

>
> Neil
>
> --
> You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
> To post to this group, send email to scintilla...@googlegroups.com.
> To unsubscribe from this group, send email to scintilla-inter...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/scintilla-interest?hl=en.
>

[Neil Hodgson]

The changes have now been committed.

Neil