Skip to content

Refactor DartDocConfig object usage, part 1 #1661

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

Merged
merged 22 commits into from
Apr 10, 2018

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented Apr 6, 2018

A first step toward making dartdoc_options.yaml play nicely with command line options. The way dartdoc handles configuration right now is very confusing -- some things are referenced in a global configuration object, some things are referenced via an object that's passed around, some things are referenced directly from parameters that are also in the config object... On top of that, Dartdoc's internal caching often had to be bypassed for testing because cached values were dependent on configuration objects.

Unfortunately, adding the dartdoc options file added Yet Another Way of configuring things and didn't tie it in to the old ways. To integrate that, we first need to get the rest of dartdoc's configuration under control.

This does that. There's only one way to look at each of the major flags now -- via a config object that is passed in to DartDoc objects at construction time. This makes it safe to, among other things, have multiple DartDoc objects with different configuration objects generate docs inside the same program (a pretty basic prerequisite for #1609).

Also:

  • Sorts parameters to the configuration object so it is easier to tell visually that things match up
  • Eliminates the global config getter, setConfig(), and so on
  • Revives a commented out test that verifies exclude flag behavior
  • Refactors LibraryContainer and subclasses so their sorting can be tested outside of actually constructing an enormous package, because the old way of avoiding this was to leverage global configuration objects.

Some of the command line options (those that involve setting up the generators) aren't moved in this PR, but will be in a followup as that's a fairly large change too.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Apr 6, 2018
@jcollins-g jcollins-g requested review from devoncarew, keertip and pq April 6, 2018 22:13
Copy link
Member

@pq pq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


@override
String get name => _name;
// Order by which this container should be sorted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> /// ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


DartDoc dartdoc = new DartDoc(inputDir, excludeLibraries, sdkDir, generators,
outputDir, packageMeta, includeLibraries, includeExternals);
DartDocConfig config = new DartDocConfig.fromParameters(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@jcollins-g jcollins-g Apr 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevmoo Hmmm. What I'm really looking for is a non-throwing, tolerant version of safe_config thrown in a blender with build_cli and a sprinkle of the analyzer's directory-specific configuration semantics. I have something like that a few PRs out on this line of development.

@jcollins-g jcollins-g merged commit 9c24147 into master Apr 10, 2018
@jcollins-g jcollins-g deleted the config-object-passthrough branch April 10, 2018 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants