Convert counter-system #defines to enum classes.
Categories
(Core :: CSS Parsing and Computation, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox77 | --- | fixed |
People
(Reporter: zech.ph, Assigned: zech.ph)
References
Details
Attachments
(2 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
2.90 KB,
patch
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•4 years ago
|
||
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•4 years ago
|
||
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!
Assignee | ||
Comment 3•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
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•4 years ago
|
||
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.
Assignee | ||
Comment 6•4 years ago
|
||
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.
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/204c30120476 Convert counter-system #defines to enum classes. r=emilio
Comment 8•4 years ago
|
||
bugherder |
Description
•