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

More doc/man1 fixes #10065

Closed
wants to merge 16 commits into from
Closed

More doc/man1 fixes #10065

wants to merge 16 commits into from

Conversation

levitte
Copy link
Member

@levitte levitte commented Oct 1, 2019

A number of replacables were still bold.
A few literals were not bold(!)
Some options were given without the initial dash
Some ellipses (especially for the -rand argument) were confusing
There was a mix of L<cmd(1)> and L<openssl-cmd(1)> references
Some references to other sections weren't in link form

All file names or parts of file names are now all wrapped with F<>
More literal input / output is now wrapped with C<>
A few engine names fixed
Corrected tsget.pod (including its file name, which was misrepresenting the command)

@levitte levitte added branch: master Merge to master branch issue: documentation The issue reports errors in (or missing) documentation labels Oct 1, 2019
@levitte
Copy link
Member Author

levitte commented Oct 1, 2019

@richsalz, feel free to give me hell 😉

doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
Copy link
Contributor

@richsalz richsalz left a comment

Choose a reason for hiding this comment

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

Whew.

Many of the comments are repeated. We should think about factoring out the engine and rand writerand options to openssl.pod common manpage. probably other flags, too.

doc/man1/CA.pl.pod Show resolved Hide resolved
doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
doc/man1/CA.pl.pod Outdated Show resolved Hide resolved
doc/man1/openssl-x509.pod Outdated Show resolved Hide resolved
doc/man1/openssl.pod Show resolved Hide resolved
doc/man1/tsget.pod Outdated Show resolved Hide resolved
doc/man1/tsget.pod Outdated Show resolved Hide resolved
doc/man1/tsget.pod Outdated Show resolved Hide resolved
@levitte
Copy link
Member Author

levitte commented Oct 2, 2019

With the find-doc-nits fixes that were recently merged, this should run fine on Travis

@richsalz
Copy link
Contributor

richsalz commented Oct 2, 2019

Please also see my comment (wrong link before) #10065 (comment)

@levitte
Copy link
Member Author

levitte commented Oct 2, 2019

Okie, all comments answered, I hope. As for ..., another PR if at all. The space before isn't absolutely necessary, but looks nicer in my opinion.

@richsalz
Copy link
Contributor

richsalz commented Oct 2, 2019

I'll assume good faith and due diligence :) Not planning to re-read this unless you ask me.

This is nice, tedious, work. Good job!

@levitte
Copy link
Member Author

levitte commented Oct 2, 2019

I won't ask anyone to read this PR again, but I do hope that a few of us will take the time to read through the man-pages and fix what needs fixing. It can be done one page at a time, or several, as it pleases each of us.

@levitte
Copy link
Member Author

levitte commented Oct 2, 2019

(I've worked as proofreader... and quite frankly, I don't like me in that mode, I can be very unkind... so I've avoided doing that with the man-pages)

@richsalz
Copy link
Contributor

richsalz commented Oct 7, 2019

I want to start working on missing content and refactoring common flags to a common manpage, and I'd rather start from this base than have either Richard or I go through a gnarly rebase. Can this get approved and merged soon?

@levitte
Copy link
Member Author

levitte commented Oct 7, 2019

I should rebase it first...

Ellipses were used to express that the '-rand' value can specify
multiple files, like this:

    B<-rand> I<file...>

Because there are conventions around ellipses, this becomes confusing,
because '-rand file...' is normally intepreted to mean that
'-rand file1 file2 file3' would be processed as three randomness
files, which makes no sense.

Rather than making things complicated with more elaborate syntax, we
change it to:

    B<-rand> I<files>
… dash

Quite a lot of replacables were still bold, and some options were
mentioned without a beginning dash.
Almost all OpenSSL commands are in reality 'openssl cmd', so make sure
they are refered to like that and not just as the sub-command.

Self-references are avoided as much as is possible, and replaced with
"this command".  In some cases, we even avoid that with a slight
rewrite of the sentence or paragrah they were in.  However, in the few
cases where a self-reference is still admissible, they are done in
bold, i.e. openssl-speed.pod references itself like this:

    B<openssl speed>

References to other commands are done as manual links, i.e. CA.pl.pod
references 'openssl req' like this: L<openssl-req(1)>

Some commands are examples rather than references; we enclose those in
C<>.

While we are it, we abolish "utility", replacing it with "command", or
remove it entirely in some cases.
"gost" was called "ccgost".
"rsax" was treated like literal input rather than an engine name.
Better synopsis for 'openssl dgst' and 'openssl enc', correct names
for 'openssl rehash' ('c_rehash' is mentioned there too), correct
option end marker for 'openssl verify', and finally, refer to
sub-commands as sub-commands.
Make replacables italic, change '-rand' to '-r', fix links.
@levitte
Copy link
Member Author

levitte commented Oct 7, 2019

Done

@t8m
Copy link
Member

t8m commented Oct 8, 2019

Unfortunately the find-doc-nits fails after the rebase.

doc/man1/openssl-s_time.pod Outdated Show resolved Hide resolved
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

I've gone through all the changes and except the few comments I did not find anything that would be a regression. There of course might be other formatting issues.

doc/man1/openssl-sess_id.pod Outdated Show resolved Hide resolved
doc/man1/openssl-smime.pod Outdated Show resolved Hide resolved
doc/man1/openssl-ocsp.pod Outdated Show resolved Hide resolved
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

Good work!

@t8m t8m added the approval: done This pull request has the required number of approvals label Oct 9, 2019
levitte added a commit that referenced this pull request Oct 9, 2019
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
Ellipses were used to express that the '-rand' value can specify
multiple files, like this:

    B<-rand> I<file...>

Because there are conventions around ellipses, this becomes confusing,
because '-rand file...' is normally intepreted to mean that
'-rand file1 file2 file3' would be processed as three randomness
files, which makes no sense.

Rather than making things complicated with more elaborate syntax, we
change it to:

    B<-rand> I<files>

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
… dash

Quite a lot of replacables were still bold, and some options were
mentioned without a beginning dash.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
@levitte
Copy link
Member Author

levitte commented Oct 9, 2019

Merged.

b1c0cc2 Command docs: fix ellipses, the easy cases
fed8bd9 Command docs: remove ellipses for '-rand'
2f0ea93 Command docs: replacables are in italics, options always start with a dash
35a810b Command docs: fix up command references
f5c14c6 Command docs: fix links to other sections (sometimes in other manuals)
1948394 Command docs: wrap literal file names with F<>
a43384f Command docs: wrap literal input/output with C<>
bc9564c Command docs: fix some engine references
b2bdfb6 Command docs: diverse small fixes
0503f08 Command docs: rename openssl-tsget.pod to tsget.pod, and fix it
8bc93d2 Command docs: more reference fixes
6f02932 util/find-doc-nits: ignore tsget.pod name

levitte added a commit that referenced this pull request Oct 9, 2019
Almost all OpenSSL commands are in reality 'openssl cmd', so make sure
they are refered to like that and not just as the sub-command.

Self-references are avoided as much as is possible, and replaced with
"this command".  In some cases, we even avoid that with a slight
rewrite of the sentence or paragrah they were in.  However, in the few
cases where a self-reference is still admissible, they are done in
bold, i.e. openssl-speed.pod references itself like this:

    B<openssl speed>

References to other commands are done as manual links, i.e. CA.pl.pod
references 'openssl req' like this: L<openssl-req(1)>

Some commands are examples rather than references; we enclose those in
C<>.

While we are it, we abolish "utility", replacing it with "command", or
remove it entirely in some cases.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
"gost" was called "ccgost".
"rsax" was treated like literal input rather than an engine name.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
Better synopsis for 'openssl dgst' and 'openssl enc', correct names
for 'openssl rehash' ('c_rehash' is mentioned there too), correct
option end marker for 'openssl verify', and finally, refer to
sub-commands as sub-commands.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
Make replacables italic, change '-rand' to '-r', fix links.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
levitte added a commit that referenced this pull request Oct 9, 2019
Normalise on L<openssl-cmd(1)> over L<cmd(1)>

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
@levitte levitte closed this Oct 9, 2019
levitte added a commit that referenced this pull request Oct 9, 2019
It's a separate script, not an openssl sub-command

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from #10065)
@levitte
Copy link
Member Author

levitte commented Oct 9, 2019

@richsalz, I believe you have some rebasing to do...

@richsalz
Copy link
Contributor

richsalz commented Oct 9, 2019

yes. :) Or is that :(

working on it, will push updated PR's soon.

@levitte levitte removed the issue: documentation The issue reports errors in (or missing) documentation label Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants