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

[Lex Trotman]

On 17 May 2011 12:17, Matthew Brush <mbrush at codebrainz.ca> wrote:
> On 05/16/11 18:05, Lex Trotman wrote:
>
>> I agree its better to load the file, but I don't know that it will get
>> less bug reports if I know users (being one) :-)
>>
>> My main concern with all these comments is to get a situation where
>> there are as few surprises as possible, surprises are what confuses
>> and upsets users.
>
> Maybe the dialog warning that pops up on non-standard text files should say
> "Warning: Unlike most text editors, Geany is going to open this file despite
> it containing questionable text.  Please do not report any bugs on issues
> occurring while this document is open." ... hahaha.

Well since 90% of the stuff edited is questionable text (a definition
that is independent of containing NULs) maybe we just try applying it
to all files :-)

>
>> And I might add that even though we see it as an improvement and are
>> thankful, users just take it as a given and go on to the next issue.
>> Sigh...
>
> Heh.  See warning above :)
>
>> Scite works because it uses Scintilla's built in but limited regex
>> engine, we use the superior system or GNU regex engine, but it only
>> takes a NUL terminated string to search.
>> I don't think this one is fixable, lets put it on the list of things
>> that don't work to be documented.  Search is limited to text only.
>
> Yeah, I can't find any info on the GNU regex stuff that indicates it can
> handle embedded NULs.  I'm pretty sure other, arguably superior, regex
> engines support it (according to BSD man pages and PCRE docs).  At this
> point though, documenting it would be best, at least until we see if there's
> a workaround.  A quick glance at Geany's source and it seems like we're
> using a mix of the built-in regex engine and the GNU/stdlib stuff, but not
> by replacing Scintilla regex backend as it suggests to do in the docs.
>  Perhaps this could be something to look at.

Later.

>
>>
>> No, cut/paste happens in Scintilla we can't do anything about it, but
>> my guess would be that it uses the GTK clipboard, and that only takes
>> NUL terminated strings.
>> Again I think, document and leave.
>
> Haven't checked the source much, but I would assume SCI_COPYRANGE and
> SCI_COPYTEXT would work since they accept positions/length.  According to
> the GtkClipboard docs, the gtk_clipboard_set_text() take a length argument
> as well.  Further study is required.
>

Yes but ScintillaGTK.cxx gets the length to pass to the
gtk_selection_set_text using strlen, oops...only part is copied to the
selection.

And there are some worrying comments about use of \0 in rectangular selections.

>>
>> So long as truncation doesn't do something destructive.
>
> I think by definition truncation is destructive :)

Heh heh, I meant silently do something destructive that the user might
not notice.

>
>>
>> So long as they don't do something unexpectedly destructive or crash.
>> You've identified the VCS plugin already.
>
> I haven't looked at much of the plugins' source, so I'm not too sure what
> they are doing.  The issues with GeanyVC and GeanyGenDoc aren't related to
> NUL chars or anything, just that they use a function for which I changed the
> signature in these changes, so they would need to be updated accordingly.
>

Ok, I havn't either.

The things that I am still worried about is code that might get a
buffer using strlen, and then it is too short for whats put in it and
that causes a crash.  Losing all your editor tabs because you opened a
file with NULs in it would be bad.

Cheers
Lex