Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Specify Include path for djinni imports #244

Closed
kennykaye opened this issue Jun 29, 2016 · 8 comments
Closed

Specify Include path for djinni imports #244

kennykaye opened this issue Jun 29, 2016 · 8 comments

Comments

@kennykaye
Copy link

Problem

TL;DR
@import statements are relative with no way to specify an include path.

Currently when importing djinni files, the @import directive only resolves files relative to the current file. For larger projects this becomes problematic as each idl must know where in the filesystem each import exists. When this location changes, all the idls must be updated individually.

Below is a rough proposal for how this problem could be addressed.

Proposed Solution

Specify an --idl-include-path which contains one path to check for an @import directive. Multiple --idl-include-path could be specified.

These path would be resolved in order specified, with the current directory taking the highest priority.

For Example

The following:

  --idl-include-path "../parent/vendor/idls"
  --idl-include-path "../parent/vendor/third-party-idl"

would resolve:

  1. ./
  2. ./../parent/vendor/idls
  3. ./../parent/vendor/third-party-idl

If a file is matched in one path, the rest of the tree will not be checked.

I would love to hear thoughts/feedback. If the team feels this is a valuable addition I would be happy to give a shot at implementing it.

@artwyman
Copy link
Contributor

I think this is a good idea. @xianwen has been putting some thought into imports recently, so may have suggestions here. I think all his current work has applied to search paths for headers after importing, not to the imported djinni files themselves, so there shouldn't be any major conflicts.

@kennykaye
Copy link
Author

@artwyman Upon further thought, it may also make sense to either extend this to @extern as well as @import.

@xianwen
Copy link

xianwen commented Jul 4, 2016

I think it's a nice to have feature, but this proposal alone might not be very helpful:

Although the idl files are being placed in other folders, currently, the generated source code still outputs to the same folder. So I don't feel we can gain much from moving just the idl files around. Unless corresponding generated files are placed in those folders as well.

@mknejp
Copy link
Contributor

mknejp commented Jul 4, 2016

It would definitely make sense for @extern since those YAML files come from other projects and are certainly not at the same location as the current idl files.

And if we add these in they should probably reflect the name of the command they are used on, i.e. it's called "include path" for #include in C, so for Djinni I suggest --import-search-path and --extern-search-path to reduce confusion.

@artwyman
Copy link
Contributor

Do imports and externs really need separate search paths? It seems it would be simpler to have a single search path for any files which can be pulled into your IDL.
Agreed with Xianwen that this feature is independent of header search paths for the generated code, which might also be needed for some use cases. I think this feature could be useful on its own, though.

@artwyman
Copy link
Contributor

Addressed in #252. Thanks @GreatCall-KayeK!

@kamalmarhubi
Copy link

@artwyman does this mean the issue can be considered solved? Reading up on Djinni right now, and figuring out how to incorporate it into a build makes this a useful feature to have.

@artwyman
Copy link
Contributor

Yes, I intended to resolve it with my prior comment. Thanks for pointing out.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants