Skip to content

docs: protect options, examples and special characters by codeblocks; create more links; apply bold style to a default value #4618

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.

Sign up for GitHub

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

Jump to bottom
Closed
pzhlkj6612 wants to merge 5 commits into from
Closed

docs: protect options, examples and special characters by codeblocks; create more links; apply bold style to a default value #4618

pzhlkj6612 wants to merge 5 commits into from

Conversation

@pzhlkj6612
Copy link
Contributor

@pzhlkj6612 pzhlkj6612 commented Jun 21, 2022

Hello!

cc1036b :

Defaults is 1.

Default is: 1.

a6280e8 :

When using -o or -r, always write to file even if it already exists (overwrite).

When using --output or --record, always write to file even if it already exists (overwrite).

When using -o or -r, show the download progress bar even if there is no terminal.

When using --output or --record, show the download progress bar even if there is no terminal.

76a910d :

Please see the "Metadata variables" section of Streamlink's CLI documentation for all available metadata variables.

Please see the "Metadata variables" section of Streamlink's CLI documentation for all available metadata variables.

1e12d02 (partial):

Usually, the protocol of http(s) URLs can be omitted (https://), depending on the implementation of the plugin being used.

Usually, the protocol of http(s) URLs can be omitted (https://), depending on the implementation of the plugin being used.

The value can contain formatting variables surrounded by curly braces, { and }. If you need to include a brace character, it can be escaped by doubling, e.g. {{ and }}.

The value can contain formatting variables surrounded by curly braces, { and }. If you need to include a brace character, it can be escaped by doubling, e.g. {{ and }}.

For example, to put the current date in your VLC window title, the string "\$A" could be inserted inside the --title string.

For example, to put the current date in your VLC window title, the string \$A could be inserted inside the --title string.

Selects a specific audio source or sources, by language code or name, when multiple audio sources are available. Can be * to download all audio sources.

Selects a specific audio source or sources, by language code or name, when multiple audio sources are available. Can be * (asterisk) to download all audio sources.

Please correct me if I changed the meaning of the docs.

@pzhlkj6612
docs: change "defaults" to "default" to apply bold style
cc1036b
@pzhlkj6612
docs: use full-length parameters to create links
a6280e8
@pzhlkj6612
docs: make cross-link to "metadata variables" in cli docs
76a910d
@pzhlkj6612
docs: protect options, examples and special chars by codeblocks
1e12d02 
- `https://` should not be interpreted as a hyperlink.
- Special characters (`{`, `}`, `*` or `-`) should be in codeblocks with theirs name.
- Inline options or theirs examples should be in codeblocks.
- Inline code examples should be in codeblocks.
- Examples of an option should contain the option itself.
@pzhlkj6612
flake8: E501 line too long
20d3c30 
./src/streamlink_cli/argparser.py:583:129: E501 line too long (130 > 128 characters)
bastimeyer
bastimeyer reviewed Jun 21, 2022
View reviewed changes
Copy link
Member

@bastimeyer bastimeyer left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

Argparse help texts can't contain any restructured text markup, because it'll make the output of the --help CLI argument look bad, as it simply outputs it as plain text. That's the reason why the ext_argparse sphinx extension applies filters via regexes (which is the best it can do), so that the rendered HTML docs and man page can contain extra markup. There's already stuff in the help texts which doesn't actually belong there.

The first three commits look fine.

The fourth and fifth should be squashed, and the reST markup get removed.

If we want to add markup to the help texts, then the output of argparse needs to be modified and the markup be removed on runtime.

@pzhlkj6612
Copy link
Contributor Author

pzhlkj6612 commented Jun 23, 2022

Hi @bastimeyer , thanks for your review.


The fourth and fifth should be squashed,

OK, I will do that.

Argparse help texts can't contain any restructured text markup, because it'll make the output of the --help CLI argument look bad, as it simply outputs it as plain text... There's already stuff in the help texts which doesn't actually belong there.

...

The fourth and fifth ... the reST markup get removed.

I agree. Codeblocks (double backticks) in help message look bad. Example:

$ streamlink --help
    ...
    The value can contain formatting variables surrounded by curly braces,
    ``{`` and ``}``. If you need to include a brace character, it can be escaped
    by doubling, e.g. ``{{`` and ``}}``.
    ...

If we want to add markup to the help texts, then the output of argparse needs to be modified and the markup be removed on runtime.

Well, removing markup on runtime doesn't sound easy...


I'm wondering that if I should split this into two pull requests. The one contains the first three commits since they look fine, and the other should be a docstring cleanup PR.

Your opinion?

@bastimeyer
Copy link
Member

bastimeyer commented Jun 23, 2022

removing markup on runtime doesn't sound easy...

It isn't even possible without fully parsing it, which we're not going to do, because not only would it be unnecessarily slow, but it would also require adding another runtime dependency.

I'm wondering that if I should split this into two pull requests. The one contains the first three commits since they look fine, and the other should be a docstring cleanup PR.

Sure, why not...

@pzhlkj6612 pzhlkj6612 mentioned this pull request Jun 23, 2022
Merged
@bastimeyer
Copy link
Member

bastimeyer commented Jun 24, 2022

Why did you open a new PR for the first three commits? I thought you wanted to update this PR and open a new one for the remaining changes. Going to close this one here then...

@bastimeyer bastimeyer closed this Jun 24, 2022
@pzhlkj6612 pzhlkj6612 mentioned this pull request Jul 17, 2022
Merged
@pzhlkj6612
Copy link
Contributor Author

pzhlkj6612 commented Jul 17, 2022

Hi @bastimeyer ,

Why did you open a new PR for the first three commits? I thought you wanted to update this PR and open a new one for the remaining changes. Going to close this one here then...

I'm sorry I thought that removing two commits from this PR would make other people confused, so I opened a new one with the commits cherry-picked from here...

Thank you for helping me close this PR.

@pzhlkj6612 pzhlkj6612 deleted the docs-links-codeblocks-styles branch October 30, 2022 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bastimeyer bastimeyer bastimeyer left review comments

Assignees

No one assigned

Labels

CLI documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pzhlkj6612 @bastimeyer