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

Matthew Brush mbrush at xxxxx
Wed May 18 00:15:13 UTC 2011


On 05/17/11 08:40, Colomban Wendling wrote:
> Le 16/05/2011 08:37, Matthew Brush a écrit :
>>
>> I've made a commit[1] to fix the issue mentioned here, which removes
>> extra regex patterns and uses two very general ones (the coding one is
>> almost the same as before).  I need to test these better still but a few
>> quick tests seem to suggest they work ok.
>
> Oops, didn't saw this one when answering to the previous one.
>
> Well, encoding detection it's better, but:
>
> 1) it could even become a single re, with "(coding|charset)"

Good point!

> 2) the space is useless in "[:= ]" since it'd be matched just after

I'll trust you on this :)

> 3) the geany_debug() call is redundant with encodings.c:357

That was an accident leaving that in there, it wasn't even meant to be 
there permanently, just for my own testing (before I realized the 
similar debug message from elsewhere).

>
> I'm just wondering whether these aren't a little too loose in real
> world, but they look OK (not like the ones of the previous patch ^^).

I had this exact same thought, but I think it's OK since it will catch 
any other types of files that may use a similar scheme, and also, AFAIK, 
it still needs to find a real encoding/charset in order to truly "work", 
so I think it's safe and flexible.  Also getting rid of that nasty 
<meta> regex would be pleasant to readers of the code :)

>
> and the changes in socket.c shouldn't be in the same commit ;)

True enough.

Like I said in my last message, I really appreciate you taking the time 
to review this code.  I'm going to get hacking on the improvements shortly.

Cheers,
Matthew Brush



More information about the Devel mailing list