Document command-line-interface of importers #507
Conversation
To decouple key links from production deploy domain
|
Deploy Preview URL: |
docs/_config.yml
Outdated
|
|
||
| defaults: | ||
| - { scope: { type: docs }, values: { layout: docs } } | ||
| - { scope: { type: importers }, values: { layout: importer } } |
There was a problem hiding this comment.
I find this JSON to be less readable than YAML.
defaults:
- scope:
type: docs
values:
layout: docsThere was a problem hiding this comment.
Now that you mention it, the definition does look like JSON. But actually, it is just YAML.
But I shall change this to the suggested format for sure. :)
| "user" => "my_username", | ||
| "api_token" => "my_api_token" | ||
| })' | ||
| {% endhighlight %} |
There was a problem hiding this comment.
The plugin you wrote is super cool and the final output is amazing! I'm kind of sad that a reader of this Jekyll site source wouldn't actually get to see much of the content that they'd view in the output. It simultaneously shows the power of Jekyll ("you can inject all sorts of content at build time!") and the drawback of that power ("a lot of the key content on the site isn't in the source files"). I wonder if it would make sense for either (1) us to generate the content into the site source which we push up to the repo or (2) the plugin to inject a variable into the Liquid template so our page can at least have a reference like {{ auto_generated_usage_guide }}. Some food for thought. Right now it's very magical but I'm debating if maybe it separates the source and final output too much?
There was a problem hiding this comment.
For hosted metadata, we could generate another data file for CLI information using a Rake task just like we have one for the list of importer-dependencies.
But that is less exciting (for me, atleast) than having content generated automatically at runtime, especially because contributors / maintainers have to ensure the data file is updated as needed.
For example, say we add a new option to an importer or we make edits to the option-description, then
- the data file has to be explicitly updated to ensure the change gets documented.
- Maintainers would have to explicitly check if the pull request has the data file updated to match the proposed change before merging the PR.
Regarding the Liquid object autogenerated..., that too is possible, but I am not yet clear on the suggested use-case.
There was a problem hiding this comment.
Good points. Let's leave it how you have it now.
| @@ -1,29 +1,13 @@ | |||
| --- | |||
| layout: docs | |||
| title: Blogger | |||
| importer: true | |||
| prev_section: behance | |||
| link_source: blogger | |||
There was a problem hiding this comment.
Side note: could we lexically alphabetize the list of importers and set these in a plugin or in Liquid too? This seems a bit inefficient to list them all here. When adding a new importer, the contributor has to know the order of their new importer in the context of other importers which we could easily get a computer to figure that out.
There was a problem hiding this comment.
Technically, this is not needed at all. At least with modern Jekyll because basically there's already the concept of next_doc and prev_doc for Jekyll::Document objects.
These were removed in the other pull request that made wholesale changes to the docs but left it untouched here to maintain the narrow scope of current PR.
|
|
||
| <p> | ||
| Sample snippet to invoke the importer: | ||
| <pre>jekyll import {{ page.cmd_name }}{% if page.cmd_opts %} {{ page.cmd_opts | map: 'switch' | join: ' ' }}{% endif %}</pre> |
There was a problem hiding this comment.
S9Y Database importer has a load of params (19, to be exact). This makes this slightly harder to see since there's so much horizontal scrolling. Is it at all possible to make this a multiline output?
jekyll import s9ydatabase \
--dbname DB \
--socket SOCKET \
...
Or do you think that would take up too much space? 🤔
There was a problem hiding this comment.
This block was just meant to quickly illustrate the CLI options. Technically for this importer, all of those options are optional. Therefore,
jekyll import s9ydatabase will work just as fine.
When making the block multiline, the end output is ugly (noisy) and we have to decide where exactly to draw the line to balance readability and accessibility. Besides, the table below lists all the options verbosely anyways.
There was a problem hiding this comment.
Ok makes sense. I thought they were all required when reading the example invocation. If there's a default value for the flag, maybe we can use the common convention of wrapping the argument in square brackets like --socket [SOCKET]?
There was a problem hiding this comment.
Modifying the long option here (i.e. bracketing the optional ARG) will cause a discrepancy between what's documented here and what gets generated in the terminal on invoking the CLI with --help switch.
Therefore, the best place to make this change is at the source-code level itself.
| @options ||= begin | ||
| @cmd.options.map do |o| | ||
| hsh = { "switch" => o.long } | ||
| if %r!(?<desc>.+?)(?:\z| \(default: (?<default_value>.*)\))! =~ o.description |
There was a problem hiding this comment.
Would be a good idea to add o.default_value or something to Mercenary, then we wouldn't need to rely on this. https://github.com/jekyll/mercenary/blob/master/lib/mercenary/option.rb
There was a problem hiding this comment.
This is indeed a good idea. But is Mercenary still actively maintained?
There was a problem hiding this comment.
It's in the Jekyll org so we can modify it to include this! It hasn't gotten love recently since it did everything we wanted, but we could take some time to improve it. 😄
|
@jekyllbot: merge +doc |
Ashwin Maroli: Document command-line-interface of importers (#507) Merge pull request 507
A slimmer version of #503:
Enhance just the dedicated page for importers.