1625745 - Convert counter-system #defines to enum classes.

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, task)

Comment 1

[Philipp Zech]

Compared to the previous changes I did, these #defines seem to work different. Unfortunately I still cannot get it to compile, as StyleCounterSystem cannot be found in servo/ports/geckolib/glue.rs as it cannot find StyleCounterSystem in structs. What is special about these #defines and how can I get them into .... by correctly generating the relevant Rust bits? thanks for any help. I manually added the import there in glue.rs, as this file does not seem to be auto-generated. I already desperately looked around for any *.mako.rs to put the relevant gecko-enum-prefix directive, but couldn't find any. But I'm anyways not really sure, whether this would fix this issue. I'm a little lost :)

PS: In case I can also submit a patch, yet, it just won't compile.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

You probably want to add them to the whitelisted-types, but hard to say without looking at your changes. Feel free to submit the patch if that doesn't work and I can take a look.

Thanks!

Comment 3

[Philipp Zech]

Attachment: Bug 1625745 - Convert counter-system #defines to enum classes. r=emilio

Comment 4

[Philipp Zech]

Thanks, that helped quite a bit, as it triggered some more errors I had to fix. But now it seems I am stuck, as it still fails to generate the bindings. I created and submitted a patch which won't compile. Thanks for your help!

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: fix.patch

Here's a fix for that. Moving the definition of CounterSystem to Rust is much easier in this case.

An alternative would be to keep your current patch and doing something like this in the cbindgen config so that cbindgen doesn't rename it, but it's not really needed. The less config needed the better.

Another bit that we should figure out is how to avoid having both StyleCounterSpeakas and StyleCounterSpeakAs. Given the first is only used in CounterStyleManager, I think moving it from nsStyleConsts.h to an enum class SpeakAs : uint8_t inside CounterStyleManager would be better, wdyt? That would avoid that confusion.

Comment 6

[Philipp Zech]

Oh thanks for that patch! This clarified some things :) dAlso, I extracted the enum you mentioned in CounterStyleManager.h and changed CounterStyleManager.cpp accordingly. You're absolutely right, this definitely resolves some confusion. Patch is submitted.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/204c30120476
Convert counter-system #defines to enum classes. r=emilio

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/204c30120476