Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use "" for local includes #52

Open
phoppermann opened this issue Mar 16, 2021 · 6 comments · May be fixed by #83
Open

Use "" for local includes #52

phoppermann opened this issue Mar 16, 2021 · 6 comments · May be fixed by #83

Comments

@phoppermann
Copy link
Contributor

e.g. in

#include <dis6/AcknowledgePdu.h>

#include <dis6/AcknowledgePdu.h> to #include "AcknowledgePdu.h"

@leif81
Copy link
Member

leif81 commented Mar 31, 2021

Thanks @phoppermann , would you like to submit a small pull request for this?

@rodneyp290
Copy link
Contributor

#include <dis6/AcknowledgePdu.h> seems to be the convention in all the dis6 & dis7 cpp files at the moment.
I'm assuming convention is actually from the initial XML generated cpp as that's the style used in the initial check in

#include <DIS/AcknowledgePdu.h>

Is there any particular reasoning for/against changing it?

I believe the current method, does require you to explicitly add ./src as a include directory (e.g. adding -I./src to the g++ command), although that is also required for the any Util includes (e.g. #include <utils/DataStream.h>)

@phoppermann
Copy link
Contributor Author

@rodneyp290 yes, it works normally. However, the include syntax with <> suggests that it's a system include (which is not the case here). See also https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html.
Apart from theoretical considerations, this can also have practical consequences e.g. in the following case: If you have installed OpenDis on your system and build it, the compiler might use the installed header instead of your own one.

@leif81 are the files generated by xmlpg or was xmlpg only used for the initial version?

@leif81
Copy link
Member

leif81 commented Apr 15, 2021

I think the use of <> is the right one. Open DIS is a library that will get installed on the system e.g. /usr/local/include/dis6/Acknowledge.h. Then in your app you would do #include <dis6/Acknowledge.h> and it just works because that's a standard system search path no?

@leif81
Copy link
Member

leif81 commented Apr 15, 2021

In the early days the source files here were repeatedly generated by xmlpg and blindly overwritten everything here, but that no longer is the case. Corrections and improvements are being made directly to the source here now.

@phoppermann
Copy link
Contributor Author

I think the use of <> is the right one. Open DIS is a library that will get installed on the system e.g. /usr/local/include/dis6/Acknowledge.h. Then in your app you would do #include <dis6/Acknowledge.h> and it just works because that's a standard system search path no?

Yes. However, that's not the point of the discussion. It's about the includes inside Open DIS.

@crhowell3 crhowell3 linked a pull request Apr 27, 2023 that will close this issue
@leif81 leif81 linked a pull request Apr 27, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants