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

docs: add dedicated metadata variables section #4207

Merged
merged 1 commit into from Nov 25, 2021

Conversation

bastimeyer
Copy link
Member

@bastimeyer bastimeyer commented Nov 24, 2021

  • add dedicated metadata variables section to the CLI docs
  • rewrite --title help text
  • update --output, --record and --record-and-pipe help texts

ref #4205

This moves the metadata variables from the argparse help texts into the CLI docs. This unfortunately means that the argparse help texts can't link to the metadata variables section. At least I don't know how to do this.

It also means that the metadata vars are currently not part of the man page anymore, because the man page only includes the argparse stuff and not the entire CLI document (because we don't want the CLI tutorial in the man page). This could be fixed in the future by splitting up the sections into their own documents and including those in the CLI document and man page.

https://deploy-preview-4207--streamlink.netlify.app/cli.html#metadata-variables
https://deploy-preview-4207--streamlink.netlify.app/cli.html#cmdoption-title

- add dedicated metadata variables section to the CLI docs
- rewrite --title help text
- update --output, --record and --record-and-pipe help texts
Copy link
Member

@gravyboat gravyboat left a comment

Choose a reason for hiding this comment

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

Doc changes look good to me, let's see if anyone spots something I missed.


Example:

%(prog)s --record-and-pipe "~/recordings/{author}/{category}/{id}-{time:%Y%m%d%H%M%S}.ts" <URL> [STREAM]
Copy link
Member

@mkbloke mkbloke Nov 24, 2021

Choose a reason for hiding this comment

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

I don't know if it's a Netlify thing only, and it's certainly very minor, but I see that different CSS is applied to and in --record-and-pipe here. I thought it was eyes at first! Technically, that's nothing to do with the content of this PR though, but it's just something I noticed and thought I would mention.

Specifically, the surrounding text is colour #d0d0d0, while and is colour #6ab825.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's because of the custom argparse extension which tries to build ReST from certain text patterns used in the argparse help texts:

_block_re = re.compile(r":\n{2}\s{2}")

# Create simple blocks.
help = _block_re.sub("::\n\n ", help)

Text indented by at least 2 spaces gets turned into a literal-block which then gets highlighted by Sphinx automatically when there's no block type specified:
https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks

There are a couple of other examples in the docs where it's parsing it incorrectly, but we can't use full ReST syntax in the argparse help texts because of the --help output and the man page, so it's a bit difficult to fix this.

Copy link
Member

Choose a reason for hiding this comment

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

It's so trivial it was not worth worrying about anyway, but more so if the fix is tricky. I just wasn't sure if it had been noticed before.

@bastimeyer bastimeyer merged commit ad9ab21 into streamlink:master Nov 25, 2021
@bastimeyer bastimeyer deleted the docs/metadata-vars branch November 25, 2021 08:54
Billy2011 added a commit to Billy2011/streamlink-27 that referenced this pull request Nov 25, 2021
- add dedicated metadata variables section to the CLI docs
- rewrite --title help text
- update --output, --record and --record-and-pipe help texts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants