On 05/18/11 17:07, Lex Trotman wrote:
That's what I meant for set_text_full(). Basically we don't need to show the real SCI api here, just what's useful to us. If it means wrapping to calls, I'd say it's just fine. And having such a wrapper will help user (here meaning "client" code) to properly give a length parameter where set_text() used to be used.
The sciwrapper functions that just wrap scintilla messages are documented by the scintilla docs.
If functions are added to sciwrapper that don't just wrap Scintilla messages or change the names then they need to be documented. Since we use doxygen comments for API generation the only option for wrappers that are not part of the API is a plain source code comment and hope people see it :-(
So if possible the sciwrapper functions should have the same name as the message. In this particular instance backward compatibility means that we can't do that, but IIUC the new function is part of the API anyway.
Hi Lex,
These are the related sci_*_text_full() commits[1][2] I'll be sending as patches to the ML once the rest of the encodings stuff is cleaned up and tested more.
The first commit adds the functions with the doc comments starting with /* for all 3 functions, and then the second commit which makes 2 of the 3 functions part of the API, makes the comments start with /** so doxygen sees them. I did it in two commits like this in case maybe these weren' to become part of the plugin API. All 3 functions are well commented (mostly wording direct from Scintilla docs).
Just wanted to update here since I didn't link to this new branch yet where I'm putting cleaner versions of the changes. If you click 'Switch Branches' then 'encodings_wip' you can see the other cleaner versions of the changes so far. Feedback welcome.
[1] https://github.com/codebrainz/geany/commit/b5a5bc171d7550561ad3c5eceaa590f1a... [2] https://github.com/codebrainz/geany/commit/cf86804594e4ab55f117e25c25adf9612...
Cheers, Matthew Brush