[Geany-devel] Highlighting setup refactoring?

[Colomban Wendling]

Le 09/11/2011 02:46, Matthew Brush a écrit :
> On 11/08/2011 02:55 PM, Colomban Wendling wrote:
>>
>> [...]
>>
> 
> These changes solve my biggest gripes with highlighting.c :)  I *think*
> there might be some performance advantages to doing it this way too
> since all of the data is static/const and known upfront.  I think the
> compiler can treat it differently now (but I'm not a GCC expert, so I
> could be wrong).

I doubt it, the compiler already knew the names (literals, so data) and
styles (contsants, easy to optimize), and the code was inlined.  Now it
uses loops, and I doubt the compiler will inline them since they are
used multiple times, so I guess there would be a small performance loss
-- though I honestly doubt it's noticeable.

I'm no a GCC/ELF expert either, just my guess.

>> What's left to do is porting DocBook, HTML, PHP and XML.
>>
>> For DocBook it is because it uses SCI_STYLESETEOLFILLED that my port
>> don't support yet (could easily with one more column);
>>
> 
> Is it possible to add the extra column, and just not use it in all the
> other arrays where it's not required?

Hehe, good idea!  C implicitly initializes to 0/NULL/whateverfits
missing members, so it's just fine.
I did it for the already existing HLKeyword.merge, and used it for the
HLStyle.fill_eol.

>> For HTML, PHP and XML it's a bit harder, because they use a weird mixed
>> setup (that I didn't try to understand too deeply yet).  IMO, first
>> thing to do would be to make each have its own styles, and inherit XML's
>> one in filetypes definitions ([styling=XML]) rather than in-code.  There
>> wouldn't be manual duplication with the proposed setup, just alias XML
>> definition to HTML and PHP.
>> Not sure what to do with the Python filetype dependency though.
>>
> 
> I *hate* how all these languages are mixed into a single lexer :)

Unfortunately I won't fix this to make Scintilla call the appropriate
lexer at the appropriate point in the appropriate file, and us set the
appropriate styles at the appropriate moment [...] :-'

>> Anyway, I'd like your opinion on this, what to improve, what you feel
>> good and bad about.
>>
> 
> Heh, I actually did a tiny bit of work (mostly thinking) about basically
> the exact same thing.  I have a script I wrote in Python that builds the
> lookup table/array code (nearly identical to yours) from
> Scintilla.iface, we could perhaps look at using that to re-generate the
> tables when Scintilla updates, or maybe even just for checking after
> updates.  I don't know if it's necessary though now that you've done the
> bulk of the tables and they don't change *that* much between Scintilla
> versions.  Just a thought.

It'd need adjustments from one side or another because there are a few
"mappings" done in the code directly, like i.e. both SCE_D_TYPEDEF and
SCE_D_WORD5 mapping to "typedef"; and as you said we anyway would need
to update some code manually at some point.

But maybe it'd be interesting, maybe it'd help not to miss any new
styles -- though actually checking the new Scintilla version's diff
should be enough.

> As we had discussed on IRC, I'd still rather not be storing duplicate
> name-mapping in (hard)code like this.  It seems cleaner to me for
> example, to have the filetypes.* containing a '[style_map]' section
> which maps named styles to scintilla style/state numbers.  The problem
> of course is that it would need comments everywhere that have the
> SCE_LANG_FOO constants, otherwise there would just be plain magic
> integers everywhere.  I guess this approach is similar to SciTE
> properties files.

I agree we better not map twice, but I don't really like the plain
integers like in SciTE .properties files -- even with the comments they
aren't that readable.
So... not sure.

> Overall I think your changes are a great improvement in maintainability
> and readability (and maybe performance).  Nice work!

Thanks :)

> BTW, should we test your current repo or wait for the other filetypes to
> be implemented?

You could've done both, there was no point where a filetype stopped
working.  Now all are ported to the new system, so you can test without
any more questions :)

BTW, I think XML/HTML/PHP might need more attention.  Although I did my
best and thin I didn't missed anything (and tried it a little), it
changed the code far more than with the other filtypes, so maybe I *did*
miss something.

Other filtypes should be just fine, it wasn't really hard to convert
(C&P, plus replacing a few things -- just a little boring) and I doubt I
made much mistakes.  However, this doesn't mean testing is not useful,
better not trust a big change like this before testing it :p


Cheers,
Colomban