Skip to content

Add capitalized headers#106

Open
LeonMatthesKDAB wants to merge 1 commit intoKDAB:mainfrom
LeonMatthesKDAB:46-capitalized-header-paths
Open

Add capitalized headers#106
LeonMatthesKDAB wants to merge 1 commit intoKDAB:mainfrom
LeonMatthesKDAB:46-capitalized-header-paths

Conversation

@LeonMatthesKDAB
Copy link

These just forward to the non-capitalized headers for backward
compatibility.

Closes #46

These just forward to the non-capitalized headers for backward
compatibility.
@seanharmer
Copy link
Contributor

Ah a macOS case-insensitive file system issue and both the source dir (kdbindings) and the new include dir (KDBindings) can cause confusion. Perhaps using relative include directives in the forwarding headers would help? I.e.

#include "../signal.h"

We'd also need to ensure the forwarding headers get put into the install location. Might even be able to get some cmake magic to generate them in the build dir like happens with the foo_export.h generated header.

@LeonMatthesKDAB
Copy link
Author

Hm, @seanharmer what kind of directory structure do you suggest?

A relative link like ../signal.h would only work if we had a structure like this:

  • src
    • kdbindings
      • KDBindings
        • sighal.h -> ../signal.h
    • signal.h

In which case both the src/ and the kdbindings/ directory would need to be include directories, which would also mean it would be possible to include signal.h directly, which is not ideal...

@seanharmer
Copy link
Contributor

Hmm right, my bad. How about if we added: <project_root>/include/KDBindings/signal.h which forward to ../src/kdbindings/signal.h? Would that work or do we still get the same case-insensitive issue arising?

If we do then I suspect the root issue is that our current source dir has the same name (case insensitive) as the include dir we desire.

If this is too much of a pain, don't waste too much time on this, we can wait and fix in version 2. It was only for consistency, it's not a blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add include paths with capitalised paths

2 participants