Config file

[pcantrell]

Here's the jazzy config file we discussed in #281. Brain dump follows.

I considered the simple approach of manually copying values out of a config file one at a time. However, this leads to a lot of redundant enumeration of option names in code already suffering from that problem, which would doubtless cause name synchronization headaches in the future.

I thus opted (←intentional pun) instead for a generalized config structure that unifies command line options, config file options, default values, and Ruby attribute declarations. (Much more DRY, but feel free to call overkill on that.)

Notes on the approach:

• I did what we discussed in [WIP] Custom categories #281: jazzy looks for a .jazzy.yaml file in the source directory, or any of its ancestors. You can override that with the --config command line option.
• All config options are available in the config file except --version and --help. Options can be config file only. This is now the case with custom_categories; the separate categories file is gone.
• Config file keys have the same names as the attr accessors in Jazzy::Config, not the command line options. (I thought they read better in that context, but could be convinced otherwise.)
• Order of option precedence is command line overrides Jazzy config file overrides podspec overrides default. However, it's necessary to read them in the opposite order of precedence: we have to parse the command line first in order to get the location of the config file, and then the config file in order to get the location of the podspec. The config thus takes a “set if unconfigured” approach.
• To this end, each foo accessor in Jazzy::Config now has a corresponding foo_configured to indicate whether it was explicitly specified in config, or merely contains a default value.
• Relative paths specified on the command line are resolved relative to the working directory. Paths in the config file are resolve relative to the file’s parent dir.
• The unified options structure made it easy to provide online help for attributes, so I did that. You can now do jazzy --help config to get help on all configuration options. You can also search for specific options, e.g. jazzy --help author
• It would be trivial at this point to add support for pulling config from env vars as well, if that's of interest.
• Same goes for generated options documentation — though the command line help is not too shabby by itself.
• I used Array(...) in this code. So there, Paul!

Here’s what the resulting config file looks like.

Things to consider:

• The config file keys are the same as the Jazzy::Config attr accessors — and I left them all as is. Since those names are being promoted from implementation detail to public configuration, it’s probably worth taking a pass to review them for clarity & consistency.
• A bunch of the command line options smell like they should be deprecated or removed (e.g. author, copyright, github_url) once the config file is available.
• As I mentioned above, paths in the config file are resolved relative to the config file’s directory. It would probably make more sense for excluded_files to be resolved relative to source_directory. I’m concerned this would be confusing, since it’s an exception; however, the excluded_files attribute is more or less useless in a config file if the file isn’t in fixed relation to the project directory (which happens if the docs are in a separate branch or repo).
• Also, since it can now appear in a config file without shell expansion, excluded_files should probably support globs.
• The behavior of the podspec option is confusing. Jazzy detects and uses values from a podspec with the default name; however, it only does the special podspec-based build if the podspec is explicitly specified, even if there’s one present with the default naming. (Do I have that right?) This option is currently undocumented, and needs documenting from somebody who understands it (i.e. not me).
• The file resolution rules open up a confusing situation: the --source-directory A command line option can point jazzy to a .jazzy.yaml file in A which then overrides source_directory to B. In that situation, jazzy would ignore B/.jazzy.yaml, because the config file’s location has already been resolved. I’d say that’s esoteric enough that we can live with it?
• Rubocop’s rules about not indenting case statements — even when they’re single-line cases — are stupid. /me shakes fist, tells kids to get off lawn

OK, that about covers it. Let me know what you think when you get a chance.

[pcantrell]

Perusing my open PRs, I noticed the CI failure on this one. The failure is due to the command line now showing “jam out to your fresh new docs in [absolute path]” instead of “…docs in [relative path].”

That probably makes more sense, given that the config can now specify a surprising output path. However, I could update the code to print the output path relative to the working directory if the former is underneath the latter.

[segiddins]

@pcantrell relative path for child directories would be preferable imho

[jpsim]

I definitely share your disdain for rubocop, @pcantrell and have occasionally made changes to .rubocop.yml. Feel free to make some changes in there rather than complying to the rules in code if you deem it appropriate.

[pcantrell]

Any thoughts on this one? Were you waiting for me to tweak Rubocop?

[jpsim]

Wow, I finally read through your original comment and through the code, very nice work Paul! Extremely thorough explanation of your choices along the way too.

I'm not too concerned about any of the points of consideration you brought up. They're mostly all existing issues rather than new problems introduced by this change.

This should be documented in the readme and added to the changelog. Otherwise, implementation-wise this looks quite nice! Some tests using a config file rather than command line arguments would be appreciated too. I think just modifying the MiscJazzyFeatures integration test to use a config file rather than the arguments would be sufficient.

[pcantrell]

Thanks! Glad it looks all right. Sorry if the thorough explanation was too thorough, but I wanted to get all those thoughts down before they wandered out of my head and off to the magical land of Important Things Paul Forgot.

Happy to add some docs, a changelog note, and tests. You could just add Siesta to your regression suite, since it now uses the config file (off my fork), but modifying MiscJazzyFeatures seems easy enough!

[jpsim]

@pcantrell you have access to the integration specs repo, actually.

[pcantrell]

I’ve added a note in the readme (which points to the newly extensive command-line help) and the changelog. I adjusted the Rubocop rules to suit my taste, but feel free to veto any of those changes.

I’ll wait to marge/rebase and to update the integration specs until #326 is merged, since I’ll have to redo them at that point anyway.

[pcantrell]

I updated the integration specs by moving the command-line options for misc_jazzy_features into a config file. While I was at it, I added custom categories to misc_jazzy_features, because it seems like that should be tested (and can be, now that there’s a config file).

AFAIK, this one is now ready to merge.

[jpsim]

This is awesome, @pcantrell! There are merge conflicts now (certainly because of #338), but those should hopefully be easy to resolve.

[pcantrell]

Merged, and I’m getting massive spec failures on some of the spec projects (though others build just fine). Errors look like this:

Could not read contents of `ROOT/tmp/document_realm_swift/RealmSwift/Aliases.swift`
Could not parse `Aliases.swift`. Please open an issue at https://github.com/jpsim/SourceKitten/issues with the file contents.
Could not read contents of `ROOT/tmp/document_realm_swift/RealmSwift/List.swift`
Could not parse `List.swift`. Please open an issue at https://github.com/jpsim/SourceKitten/issues with the file contents.
Could not read contents of `ROOT/tmp/document_realm_swift/RealmSwift/Migration.swift`
Could not parse `Migration.swift`. Please open an issue at https://github.com/jpsim/SourceKitten/issues with the file contents.

I’m investigating, but that error message ring a bell for anyone else?

[pcantrell]

Figured it out. Just a matter of the integration specs submodule getting out of sync during my merge.

I think this is once again ready to merge — and I do hope we can merge it soon, because keeping this branch up to date is getting quite unwieldy!

[jpsim]

👍 woohoo 🎉

[pcantrell]

💃🏻🎂🍻🎤 … though I’m sure that years from now, I’ll look back wistfully on the halcyon days of keeping this branch up to date.

Maybe.

[jpsim]

I know it's not always pleasant, or trivial, to keep your PRs up to date, but I really appreciate the work you put in!!

[pcantrell]

And likewise. Maintaining a project (OSS or not) is a lot of work. Everyone wants to ride the horses, but nobody wants to clean the stables!

[ad-van-erp]

I've been using the custom_categories section within the .jazzy.yaml for some weeks now, and I'm very enthusiastic about it. I could not derive from the documentation however, if name and children were the only allowed tags herein. I hoped there would also be a readme tag (following the conventions for this tag at top level), where I could store information about the category involved, but so far I haven't found it.

Maybe I overlooked something, otherwise I at least would be very pleased if you could add it in some future release.