openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.98k stars 2.55k forks source link

move libs/json/include/json.hpp to libs/json/include/nlohmann/json.hpp #8082

Closed 2bbb closed 3 months ago

2bbb commented 3 months ago

when other libraries has json.hpp, #include "json.hpp" can't load nlohmann::json.

i think moving include/json.hpp to include/nlohmann/json.hpp and replace #include "json.hpp" to #include "nlohmann/json.hpp" or #include <nlohmann/json.hpp>

for reference: #include <json.hpp> works sometime. but i think this is not complete solution, maybe...

oxillo commented 3 months ago

👍

MSYS2 is already using #include <nlohmann/json.hpp> in ofJson.h

#if !defined(TARGET_MINGW)
    #include <json.hpp>
#else
    #include <nlohmann/json.hpp> // MSYS2 : use of system-installed include
#endif

Also, examples from nlomann site are using #include <nlohmann/json.hpp>. So, I think this is the right move.

2bbb commented 3 months ago

this fix need to use apothecary...? if so, i don't know apothecary enough, please fix anyone! ☺️

danoli3 commented 3 months ago

Can do

danoli3 commented 3 months ago

https://github.com/openframeworks/apothecary/pull/429 done

Should work automatically as include location doesn't change

dimitre commented 3 months ago

Nice! should this should be changed also? in ofJson.h

#if !defined(TARGET_MINGW)
    #include <json.hpp>
#else
    #include <nlohmann/json.hpp> // MSYS2 : use of system-installed include
#endif
danoli3 commented 3 months ago

Yes if someone could make the PR for that once the releases deployed to merge

On Mon, 19 Aug 2024 at 03:06, Dimitre @.***> wrote:

Nice! should this should be changed also? in ofJson.h

if !defined(TARGET_MINGW)

include

else

include <nlohmann/json.hpp> // MSYS2 : use of system-installed include

endif

— Reply to this email directly, view it on GitHub https://github.com/openframeworks/openFrameworks/issues/8082#issuecomment-2295327854, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGK2HGL25MUCXVMA4NAH2DZSDH7RAVCNFSM6AAAAABMWH42ROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJVGMZDOOBVGQ . You are receiving this because you were assigned.Message ID: @.***>

danoli3 commented 3 months ago

Nice I think this changed exposed a few major automation test problems! XD