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

Colomban Wendling lists.ban at xxxxx
Tue May 17 15:40:51 UTC 2011

Le 16/05/2011 08:37, Matthew Brush a écrit :
> On 05/15/11 20:45, Matthew Brush wrote:
>> The second one is a simple refactoring of the code around the
>> regex-based encoding detection and adding a few more patterns to detect
>> a few more types of files. Especially needing review are the actual
>> pattern strings I used since, quite frankly, I suck at regexps. One
>> thing I noticed looking at the bug report here[5], after creating these
>> patches/commits, is that it seems the <?xml ... encoding="" ?> is
>> already supported by the CODING regex, so you can ignore the new XML one
>> I added if that truly is the case (I'll remove it from the proper patch
>> I send in the future). Like I said, my regex skills aren't great :)
> 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)"
2) the space is useless in "[:= ]" since it'd be matched just after
3) the geany_debug() call is redundant with encodings.c:357

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

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


More information about the Devel mailing list