[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