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

[Matthew Brush]

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.

> 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.

>
> 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.

>
> So long as truncation doesn't do something destructive.

I think by definition truncation is destructive :)

>
> 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.

> Well the main reason for the read-only is the truncation, so you don't
> write a short file over the long one.  Now you've fixed that, but I
> still think its worth leaving it read-only.

Agreed.  The user can always choose to ignore the warning message, set 
it read/write and modify it anyway.

Cheers,
Matthew Brush