[Geany-devel] [CODE REVIEW] Changes to encodings

[Lex Trotman]

Hi Matthew,

Comments from inspection only as I am still development machine
challenged.  Also only on the NUL handling, encodings are not my
specialty (as in I avoid them like the plague).

> not such a big deal).  Also, I'm not completely satisfied with the name of
> the sci_add_text2() function added to the sciwrappers files, but there was

Seems ok to me, it adds text too :-)

[...]
> The following bug reports are related to the first set of changes:
> [1][2][3][4].  A few of the comments seem to suggest that it's not desired
> to support text files containing NUL control bytes, but IMO, it's nice to be
> able to show these anyway, especially since Scintilla has a nice way of
> dealing with these.  IMO, as long as the file gets marked read-only,
> displaying as much as possible is better than truncating at the first NUL
> byte.

I don't think anyone is actively *against* handling NULs, just against
the work involved for little gain.  But of course if you think its
more important then ... oh you have :-)


> If you want to use the GitHub UI to review the commits [6][7] otherwise,
> patches are attached.  If you have a GitHub account, you can leave comments
> on the commits and/or specific lines of the commits.
>
> Any feedback would be much appreciated as I plan to submit these changes as
> proper patches once I've ensured they are good enough.
>

The problem I see with opening files with embedded NULs is then
ensuring that it doesn't break when code that expects NUL termination
accesses the buffer.

1. editor.c and others including plugins use sci_add_text a number of
times, will any of that break? If so it needs to be disabled or fixed.
2. do searches work with files containing NULs?
3. How do selections work with embedded NULs, AFAICT all the selection
calls use NUL termination?
4. All the places sci_get_string, sci_get_line, sci_get_text,
sci_get_contents and any others that get/set text using NUL
termination need to be disabled or fixed.
5. All of the above goes for plugins too.
6. Do lexers work with embedded NULs or does such a file have to be
filetype None, if so it needs to be enforced.

Cheers
Lex