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

Update man pages based on avifenc/dec's --help message. #2582

Merged
merged 4 commits into from
Jan 27, 2025

Conversation

maryla-uc
Copy link
Collaborator

Try to minimize differences with the --help message. Ideally, any change should be made in both, unless it's man-page specific formatting.
Do not include deprecated flags (--min/--max etc.) Don't impose any line length limit in the markdown files to minimize work.

Fixes #2534

Try to minimize differences with the --help message.
Ideally, any change should be made in both, unless it's man-page
specific formatting.
Do not include deprecated flags (--min/--max etc.)
Don't impose any line length limit in the markdown files to minimize work.

Fixes AOMediaCodec#2534
@wantehchang
Copy link
Collaborator

Maryla: Thank you very much! I will take a look.

@maryla-uc
Copy link
Collaborator Author

FYI I started over from the --help message and markdown-ified it, then made a few changes or reverted some. But I did remove some nicer formatting in the man page for the sake of matching the --help message. Let me know if you're ok with this approach or if I should revert more stuff.

@wantehchang
Copy link
Collaborator

Maryla: This is a good approach. I will review the new man pages by comparing them with the help messages.

Copy link
Collaborator

@wantehchang wantehchang left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot!

apps/avifenc.c Show resolved Hide resolved
apps/avifenc.c Show resolved Hide resolved
doc/avifenc.1.md Outdated Show resolved Hide resolved
doc/avifenc.1.md Outdated Show resolved Hide resolved
doc/avifdec.1.md Outdated Show resolved Hide resolved
doc/avifdec.1.md Outdated

: - **8**
- **16**
: Output depth [8,16]. (PNG only; For y4m, depth is retained, and JPEG is always 8bpc).
Copy link
Collaborator

Choose a reason for hiding this comment

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

we have some inconsistency with how we note intervals and sets. How about {8,16} for a finite set of values and (0..100) (WebP notation in the command line) for an interval?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will leave this as is until we have a decision or for some future PR :)

: Decode all frames and display all image information instead of saving to disk.

**\--icc** _FILENAME_
: Provide an ICC profile payload (implies \--ignore-icc).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which style do we choose ? Provides or Provide ? Below, it is Specifies. The usual where this parameteris implied?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. I removed a bunch of "set" and "provide" "specify" except where I thought it still made sense. There's probably still room for improvement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://www.gnu.org/prep/standards/html_node/Man-Pages.html#Man-Pages and https://man7.org/linux/man-pages/man7/man-pages.7.html do not seem to address this issue. (I searched for the word "verb" in those two pages and didn't find anything.)

I suggest we follow the convention in the Unix man pages, which seem to use Provide rather than Provides. Here are three examples:

doc/avifdec.1.md Outdated Show resolved Hide resolved
doc/avifenc.1.md Outdated Show resolved Hide resolved
doc/avifenc.1.md Outdated
If neither duration nor timescale are set, **avifenc** will attempt to use
the framerate stored in a y4m header, if present.
Default is 30.
: Set the timescale to V. If all frames are 1 timescale in length, this is equivalent to frames per second (Default: 30)
Copy link
Collaborator

Choose a reason for hiding this comment

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

to _V_

doc/avifenc.1.md Outdated Show resolved Hide resolved
@vrabaud
Copy link
Collaborator

vrabaud commented Jan 24, 2025

@jzern , what's your take on interval/set notations? From previous discussions for WebP, I believe we chose:

- [a,b] for an inclusive interval of floats, [a,b) to exclude
- (a..b) for an inclusive interval of ints
- {a,b} for a set containing only a and b

@wantehchang
Copy link
Collaborator

[Maryla: This comment is a discussion, not a request for change.]

Re: Vincent's question: For integers we can omit the parentheses and square brackets. The following would be my suggestion:

  • [a,b] for an inclusive interval of floats, [a,b) to exclude b
  • a..b for an inclusive interval of ints
  • a, b or a and b for a set containing only a and b

apps/avifenc.c Show resolved Hide resolved
doc/avifdec.1.md Outdated Show resolved Hide resolved
: Decode all frames and display all image information instead of saving to disk.

**\--icc** _FILENAME_
: Provide an ICC profile payload (implies \--ignore-icc).
Copy link
Collaborator

Choose a reason for hiding this comment

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

https://www.gnu.org/prep/standards/html_node/Man-Pages.html#Man-Pages and https://man7.org/linux/man-pages/man7/man-pages.7.html do not seem to address this issue. (I searched for the word "verb" in those two pages and didn't find anything.)

I suggest we follow the convention in the Unix man pages, which seem to use Provide rather than Provides. Here are three examples:

@maryla-uc maryla-uc merged commit ce8b972 into AOMediaCodec:main Jan 27, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

man pages are outdated
3 participants