typesafe API

[Stefan Küng]

So I changed all my code to use the type safe ScintillaCall API. I'd like to make some observations if you don't mind:

• There are two GetCurLine() methods: one where a buffer and length is passed, and one where only length is passed. The second one doesn't make any sense because the memory is allocated inside the method, but it still requires to first get the buffer size.
• Scintilla:FoldLevel enum is missing -1 (see SCI_GETLASTCHILD, where -1 can be used)
  
• some enums that are bitmasks are missing either | or & operators (or even both), e.g. AutomaticFold, EOLAnnotationVisible, ...
• Scintilla::LineEndTypeSupported() should return Scintilla::LineEndType, not int
  
• Scintilla::Alpha could use an (int) operator so passing an alpha value between 1-254 is possible without casting. You could even assert if the value passed is not in the range 1-254.
  

apart from that, I really like it!

Stefan

[Neil Hodgson]

Stefan Küng:

> • There are two GetCurLine() methods: one where a buffer and length is passed, and one where only length is passed. The second one doesn't make any sense because the memory is allocated inside the method, but it still requires to first get the buffer size.

APIs with a stringresult parameter produce 2 methods: one that returns a std::string and one that takes a char* and returns a size. This allows clients to reuse a buffer if they want to avoid allocations.

Other stringresult methods place a selecting argument (like a style index in StyleGetFont) in the first parameter and the binding generator doesn’t know that the length argument to GetCurLine is special. GetCurLine also returns the caret position within the line, not the line length when called with a non-NULL second argument. Its a mess and should probably just be deprecated.

> • Scintilla:FoldLevel enum is missing -1 (see SCI_GETLASTCHILD, where -1 can be used)

Its not really a fold level as it its only accepted by SCI_GETLASTCHILD. Including -1 as a value in FoldLevel could lead to it being passed to SetFoldLevel or ExpandChildren where it won’t have a sensible effect.

> • some enums that are bitmasks are missing either | or & operators (or even both), e.g. AutomaticFold, EOLAnnotationVisible, ...

Operators are defined manually when they appear worth the effort. There’s also an |= and some other helpers like LevelNumberPart. | and & could be defined as templates but that applies them too broadly, defeating some of the benefit of using enum class. I generally prefer using the FlagSet function more than defining & unless the expressions become too complex. There could be extra data in the iface file to show which enums are bitmasks or which should have | or & generated.

EOLAnnotationVisible is irregular (3 independent values then a 3x3 matrix of end types) and is better understood as an enumerated list, not bitmasks although the 3x3 ’stadium’ variants were arranged to allow for adding more end types.

> • Scintilla::LineEndTypeSupported() should return Scintilla::LineEndType, not int

OK.
https://sourceforge.net/p/scintilla/code/ci/d35f2d968f32fc7ac98ce6734caaa0259a43a42f/

> • Scintilla::Alpha could use an (int) operator so passing an alpha value between 1-254 is possible without casting. You could even assert if the value passed is not in the range 1-254.

Its an enum class which can’t have a constructor so I don’t know how to write that.

A braced initialization like Alpha{30} may be better and passes C++ Core Guidelines. A function style cast like Alpha(30) will work but will be flagged by some linters.

> apart from that, I really like it!

Good.

Neil