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

Allow use conditional directives at least within the section of the import/export directives #5

Open
mezoni opened this issue Mar 5, 2015 · 5 comments

Comments

@mezoni
Copy link

mezoni commented Mar 5, 2015

// Directives import/export
// Here allowed conditional directives

// Declarations
// Here not allowed conditional directives
library name

#if env_constant expression
// Directives import/export
#elif other_env_constant expression
// Directives import/export
#else
// Directives import/export
#endif
library name

#if dart_platform == "server" || dart_platform == "browser"
import "common.dart";
#else
#error Unsupported platform
#endif

#if dart_platform == "server"
import "server.dart";
#elif  dart_platform == "browser"
import "browser.dart";
#else
#error Unsupported platform
#endif
@munificent
Copy link

This is effectively a separate proposal, so I don't think it belongs here as a bug.

@mezoni
Copy link
Author

mezoni commented Mar 5, 2015

This is an alternative to the proposal.
I do not think that the main purpose of the "configured-imports" is just to have them as they are proposed here.
The main reason is satisfy the requirement to "import by condition".
What means word "configured" in your proposal?
Condition directives allows "to configure imports".
I don't think that your proposal have some advantages over the "conditional directives".
Parsing constant expression in the directive is not harder than parsing that what you propose.
I am talk about the multi-branch analysing.
If analyzer detect an unknown identifiers and a compared values then it also can warn about that.

Eg.

#if some_predefined_id == expected_literal
#endif

No problem. If analyzer cannot recognize a "predefined_id" as an identifier that it expected to find here then it can send warning "Cannot recognize id. Analysys would not be performed for this condition".

@mezoni
Copy link
Author

mezoni commented Mar 5, 2015

I can wrote it for you if don't know how to implement it.
I am about the detection of "multi-branching" for Dart analyzer.
You should only specify the proposed identifier, their values and what this combination means.

Eg.

dart.platform:
  values:
   "browser" # Browser engine
   "server" # Server engine
dart.vm:
  true: # Dart VM present, maybe in the browser
  false: # Not native engine, maybe javascript

@lrhn
Copy link
Owner

lrhn commented Mar 19, 2015

We could allow compound conditions in the import.
If we also have conditional exports, it's not technically necessary since you can import something if test1 && test2 by using test1 to conditionally import something that uses test2 to conditionally export what we want. It requires more library files, but it is possible.
The problem with expressions is that it's hard to stop. Using "!", "&&" and "||" to combine tests and parentheses to group them is an easy extension to this proposal, but to begin with, I'd like to keep things simple. If you have a complex condition for importing a library, there might be an abstraction missing somewhere.

@zoechi
Copy link

zoechi commented Mar 19, 2015

Is there some agreement what values are available for the comparsion in the expression?
Somewhere features where mentioned, is this still part of this DEP?
I poked around in some Ruby code a few days ago, they have a lot of if version >= x ...
If it is only platform, the mentioned operators should do.
Of course someone could still demand other operations like platform.startsWith('xxx'), platform.contains((math.pi * 42).toString()), ... or even http.get()...contains(...) ;-)

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

No branches or pull requests

4 participants