s00500 / ESPUI

A simple web user interface library for ESP32 and ESP8266
https://valencia.lbsfilm.at/midterm-presentation/
Other
932 stars 174 forks source link

Conflicting Enums in global scope #290

Open vbartusevicius opened 10 months ago

vbartusevicius commented 10 months ago

It is impossible to use s00500/ESPUI and dirkhillbrecht/UiUiUi in a single project because both define global constants.

.pio/libdeps/nodemcu/UiUiUi/src/UIEnums.h:12:20: error: 'None' conflicts with a previous declaration
   12 | enum UIExpansion { None,Horizontal,Vertical,Both };
      |                   ^~~~
In file included from .pio/libdeps/nodemcu/ESPUI/src/ESPUI.h:22,
                 from lib/WebAdmin/WebAdmin.h:7,
                 from src/main.cpp:7:
iangray001 commented 10 months ago

Yeah, C is kinda dumb with this. There are of course ways it can be fixed but it would be a huge breaking change. @MartinMueller2003 is there a way to switch a project to scoped enums without breaking downstream projects?

MartinMueller2003 commented 10 months ago

Not really. You would want to push the enums into a class to make them class local. I suspect now one is using the correct annotation EnumName::Value and that is why there is a conflict. The same can be said for the other application. They have put the enums in the global scope instead of in a class public scope and that causes issue like this.

If we do this then people will have a breaking issue but the fix is simple. Add

ContainingClass::EnumName:: in front of the enum value usage. That is not a big change (ie PlatformIO can do a show all references making it easy to change). Additionally, Instances in the code will get quickly highlighted by compilation failures.

vbartusevicius commented 10 months ago

Added same issue to the other repo - https://github.com/dirkhillbrecht/UiUiUi/issues/14

MartinMueller2003 commented 9 months ago

Any decision here? I would go for the breaking change because it is the right way to do this.

s00500 commented 9 months ago

Hm... pretty frustrating... do we have any chances of adding at least editor highlights for the break ? otherwise we could do a version bump and add a little migration guide for this...

MartinMueller2003 commented 9 months ago

I would say we bump and create an upgrade guide. I would also suggest that a lot of other global structures currently in place should become private with accessors created as needed.

s00500 commented 9 months ago

I think I would be ok with that if you start working on a PR :-)