@codebrainz 

There are plenty of tools out there that can remove NULs if that is actually 
the correct thing to do to the file (which its probably not if its an incorrect 
encoding).  So its not a good trade-off of effort vs return to add the 
capability to Geany for an occasional faulty file.  Just because its possible 
to do, doesn't mean it should be done.

> Have you actually checked how many functions used in Geany would be affected? 
> I haven't, but I ask because it might be worth actually checking to see, 
> maybe it's not so many more as one might assume.

I don't support adding editing so I won't do that. :)

@b4n 

> some encodings might generate NUL bytes in their encoded stream anyway (or at 
> least could anyway)

Sure, for example an ASCII file saved in UTF-16 will be full of NUL bytes, and 
it would be a bug if we did not allow for that, but thats different to NUL in 
the UTF-8.  (and saving a file with "foo" as UTF-16 indeed works fine despite 
every second byte being NUL :)

> Anyway: what I fixed was simply that now we properly write the whole thing in 
> any case, with no corner case doing otherwise.

Ok, a silent truncation on saving of a buffer with a NUL is a "bad thing" :tm: 
, for example if a buggy plugin made a NUL.

> Non-regex search doesn't, however (could be interesting to fix, but for now 
> it's out of the scope of this).

So long as there have been comprehensive tests that __nothing__ that accesses 
the readonly buffer with NULs can (1) crash (2) hang (3) cause problems for 
other buffers (and that includes testing all plugins).

Note I didn't say anything had to work right, just not cause problems.  

> So @elextr I think the goal of this PR matches your hopes: loading files, but 
> nothing else -- and warning users it's not properly supported and can cause 
> issues.

And to be clear I support that if there is a proposal for how to do the 
comprehensive testing I mentioned above, and will review this when I get a 
chance.

> But if one day we end up with full support for those, I don't think it would 
> be a bad thing.

And as I said above, just because we __can__ doesn't mean we __should__ waste 
time on it when there are many other things to be done.  And don't forget 
plugins.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/1709#issuecomment-349472427

Reply via email to