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

Update optionsProcessed API to pass "raw" maps. #25126

Closed
pq opened this issue Dec 7, 2015 · 5 comments
Closed

Update optionsProcessed API to pass "raw" maps. #25126

pq opened this issue Dec 7, 2015 · 5 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. customer-flutter P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@pq
Copy link
Member

pq commented Dec 7, 2015

Currently the expectation is that we pass options of type Map<String,YamlNode> which is not compatible with the type of map we get when merging embedded options with user-defined ones.

This blocks: #25115.

@pq pq added type-enhancement area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. customer-flutter labels Dec 7, 2015
@pq pq self-assigned this Dec 7, 2015
@bwilkerson
Copy link
Member

Is there any way to get better type information? Should we pre-process the YamlNodes into something that is compatible?

@pq
Copy link
Member Author

pq commented Dec 7, 2015

Is there any way to get better type information? Should we pre-process the YamlNodes into something that is compatible?

I've puzzled about this but I'm not sure. The rub is that yaml (and the options format) is so open-ended. For example, a valid value might be a string, bool, list or map. In the end, I don't see any way to sensibly process options without a bunch of is checks. I'm not sure there's that much value in passing in anything other than Map<String,dynamic>. Needless to say, thoughts appreciated!

@bwilkerson
Copy link
Member

Aside from a general like for type annotations :-) I'm a bit concerned that processors will end up needing to be written like this:

if (value is YamlNode) {
  // logic for parsing raw yaml nodes
} else if (value is Map) {
  // nearly identical logic for parsing a map
}

because it has to handle both raw inputs and merged inputs. I hate the idea of having to duplicate large amounts of logic. Maybe a wrapper around one form or the other that makes them look the same?

@pq
Copy link
Member Author

pq commented Dec 7, 2015

:)

I feel you. I believe that YamlMaps can be treated nearly as simple maps for the purposes of processing. IOW: I think we can get away with only a few checks. I'll explore this though and follow-up.

pq added a commit that referenced this issue Dec 7, 2015
Updates processing API to accept untyped options maps.

Required to support processing options merged from embedders and local analysis options.

See: #25126

[email protected]

Review URL: https://codereview.chromium.org/1506963002 .
pq added a commit to dart-lang/linter that referenced this issue Dec 7, 2015
Rationale here: dart-lang/sdk#25126

Note the TODO in `parseConfig`.  If we don't replace the untyped options map with something typed, we'll want to come back and unify the divergent parse implementations.

BUG=
[email protected]

Review URL: https://codereview.chromium.org//1508893002 .
@pq
Copy link
Member Author

pq commented Dec 7, 2015

Analyzer API: ee310b6.
Linter update: dart-lang/linter@97a27c3.
Linter and error filter config fixes in flight: https://codereview.chromium.org/1503353002/

@pq pq closed this as completed Dec 7, 2015
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed Priority-Medium labels Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. customer-flutter P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants