ompi icon indicating copy to clipboard operation
ompi copied to clipboard

Documentation beautification on the rst pages

Open zerothi opened this issue 2 years ago • 18 comments

Is your feature request related to a problem? Please describe. The new documentation produces some weird looking links on when viewed on a website

A reference: https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Win_allocate.3.html#mpi-win-allocate

Describe the solution you'd like In the above link I would like to:

  1. Change ERRORS.rst to list form of the
    Custom MPI error handlers can be created by calling: [MPI_Comm_create_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Comm_create_errhandler.3.html#mpi-comm-create-errhandler) [MPI_File_create_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_File_create_errhandler.3.html#mpi-file-create-errhandler)[MPI_Session_create_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Session_create_errhandler.3.html#mpi-session-create-errhandler) [MPI_Win_create_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Win_create_errhandler.3.html#mpi-win-create-errhandler)
    
    Predefined and custom error handlers can be set by calling: [MPI_Comm_set_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Comm_set_errhandler.3.html#mpi-comm-set-errhandler) [MPI_File_set_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_File_set_errhandler.3.html#mpi-file-set-errhandler) [MPI_Session_set_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Session_set_errhandler.3.html#mpi-session-set-errhandler) [MPI_Win_set_errhandler(3)](https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Win_set_errhandler.3.html#mpi-win-set-errhandler)
    
    the above items would become a list of links (easier for the eye)
  2. In the See Also section I would also like to make the entries a list in that case.
  3. Try and fix the missing spaces in the Notes section.

I would not start these changes without confirmation, let me know if you think these changes would apply all-over the rst documentations. Also, if you want all in one PR? :)

  • [x] Beautification of ERRORS.rst
    • [x] Listify the elements
    • [x] add links to the routines listed
    • [x] Remove (3) in function links
  • [x] Listify See also sections
  • [x] Add missing links across the man-pages
  • [ ] Make constants and other code use backticks to highlight code pieces
  • [ ] Once #11359 is completed, make links to the constants page (likely will be a separate PR)
  • [x] Remove references to "Sun Fortran" -- they can probably just be references to Fortran these days (one of the main initial authors of the nroff man pages was a docs writer at Sun)
  • [ ] Make linking to man pages easier, per ideas like https://github.com/open-mpi/ompi/pull/11367#issuecomment-1414297031
  • [x] Move section 5 man pages to more appropriate sections (perhaps 7?)
  • [x] Copy code examples from deprecated routines to new (see MPI_Type_struct and MPI_Type_create_struct)

zerothi avatar Jan 24 '23 13:01 zerothi

I agree -- ERRORS.rst can definitely be made a bit more beautiful.

In addition to what you suggest, I'd also turn the verbatim section under "Open MPI includes three predefined error handlers that can be used:" into a bullet list as well -- I'm not sure why it's a verbatim section.

I'm sure you're aware, but just to make sure we're on the same sheet of music: the verbatim markdown section you posted in the PR description won't be sufficient -- you'll need to use RST syntax, not MD syntax, and use relative links to link to the man pages, etc.

jsquyres avatar Jan 31 '23 16:01 jsquyres

I agree -- ERRORS.rst can definitely be made a bit more beautiful.

In addition to what you suggest, I'd also turn the verbatim section under "Open MPI includes three predefined error handlers that can be used:" into a bullet list as well -- I'm not sure why it's a verbatim section.

I'm sure you're aware, but just to make sure we're on the same sheet of music: the verbatim markdown section you posted in the PR description won't be sufficient -- you'll need to use RST syntax, not MD syntax, and use relative links to link to the man pages, etc.

Agreed, of course rst syntax ;) I'll have a jump!

zerothi avatar Feb 01 '23 10:02 zerothi

In addition, is the MPI_Comm_create_errhandler(3) <MPI_Comm_create_errhandler> really necessary? Would it be ok to remove (3)? I don't see it adding any information that isn't already present in the toc? It is clearly under man3.

zerothi avatar Feb 01 '23 10:02 zerothi

I'll add a list of items to the main post, then you can comment as we go by, I'll probably stumble upon more things ;)

zerothi avatar Feb 01 '23 10:02 zerothi

In addition, is the MPI_Comm_create_errhandler(3) <MPI_Comm_create_errhandler> really necessary? Would it be ok to remove (3)? I don't see it adding any information that isn't already present in the toc? It is clearly under man3.

FWIW, we generally tried to list the man page section number in links to man pages in all the RST docs, which serves two purposes:

  1. It emphasizes that the content we're referring to is another man page
    • E.g., it's helpful when you're reading the content via man MPI_Whatever (e.g., in a terminal), not in a pretty web page. Since the terminal-rendered man page won't have a link to click on, the (x) in the text gives the reader the clue that we're referring to another man page that they can see via man MPI_That_thing.
  2. It shows which section the man page is in

Also FWIW: We didn't make up that convention; we took that convention of including the (x) in the reference from other man pages.

jsquyres avatar Feb 01 '23 16:02 jsquyres

But that is not enforced on any other references in that man3 section? If you want, then I would argue it should also be on all seealso references etc, no?

zerothi avatar Feb 01 '23 17:02 zerothi

Yes, we should definitely be consistent in all references to man pages -- inline in text, in man pages, in see also, etc.

I'm just about to comment on the PR, but I'll say it here, too: we converted the man pages from wildly-inconsistent, handwritten nroff (which you can still see in the v4.1.x series) to RST via some scripting. We thought we did a pretty good job of catching most corner cases, but you have fixed a whole boatload more corner cases that our scripting fixed.

I.e., if you see inconsistencies like this -- particularly in the man pages -- it's almost certainly due to the initial wildly-inconsistent nroff source code for the man pages (which were written by different people at different times, so things weren't always uniform across all the man pages).

jsquyres avatar Feb 01 '23 17:02 jsquyres

I think this is just hurting the experience for those who use the web-documentation.

I would argue/think that most people use your web-page documentation and people viewing these pages will have no clue why the (3) is present. Rather few people will use the man pages. Just a guess ;)
Again I think this should be fixed via some mechanism in the conf.py. How exactly, I am not sure. But having (3) all over I must confess is just annoying ;)

zerothi avatar Feb 01 '23 19:02 zerothi

Ok, I can buy that argument.

It would be awesome if we could have it render differently in HTML and nroff. Would it be possible to make an OMPI-specific macro/function/something (I don't know the correct RST / Sphinx terminology here) for linking to man pages? This would allow us to have a compact/easy notation in the .rst files, and it renders appropriately (without the (x) for HTML, and with the (x) for nroff)?

jsquyres avatar Feb 02 '23 15:02 jsquyres

I agree this approach would be the way to do it. See my small snippet and idea in the PR.

zerothi avatar Feb 02 '23 20:02 zerothi

Looks good. Since you took #11367 out of draft, I'm guessing you want to merge that now and continue work in a new PR. I took the liberty of adding a few more tasks up in the top-level description on this PR.

jsquyres avatar Feb 03 '23 14:02 jsquyres

Re-opening because there's more to do beyond #11367

jsquyres avatar Feb 03 '23 14:02 jsquyres

I'll create a new PR against master with some of the other tasks, once they are done, I'll work on the cherry-picks.

zerothi avatar Feb 08 '23 11:02 zerothi

@jsquyres you added the item: move section5 man pages to section 7

  1. MPI_T.5.rst discusses C-routines while in the context of something broader. However, they are still C-interfaces and as such belong under man3
  2. Open-MPI.5.rst is a mixed file. It contains error-codes (which belong under man3), but also generic OpenMPI information (which agreed belongs under 7)

I can do 1., but I think 2. requires a PR on its own where we should discuss which action to be taken.

So which is

zerothi avatar Feb 08 '23 11:02 zerothi

@jsquyres as for the "Sun Fortran" notification. Would you recommend we entirely delete the Fortran 77 notes section? See e.g. https://docs.open-mpi.org/en/v5.0.x/man-openmpi/man3/MPI_Comm_get_attr.3.html#fortran-77-notes

The entire section only has meaning for the Sun Fortran compiler.

This would also mean that we need to change the header at Fortran Syntax.

Just so we agree ;)

zerothi avatar Feb 08 '23 11:02 zerothi

Another question. We have some deprecated functions.

Would it make sense to move documentation into the new functions and remove all from the deprecated routines? I think this is important to point users towards the new documentation. Currently, some of the older routines have more documentation than the replacement routines, and some replacement routines have nothing!

zerothi avatar Feb 08 '23 13:02 zerothi

Answers to your above comments:

  1. Should we split up MPI_T.5.rst and/or Open-MPI.5.rst into multiple pages that go into their respective sections?
    • I see that we have individual man pages for all the MPI_T API functions in section 3 already. What is there in MPI_T.5 seems to be general information, even though it makes passing reference to the API calls. Should it go to section 7?
    • I agree about the errors list in Open-MPI.5. Perhaps the error constants list should be its own page in section 3, and move the rest of the intro stuff in section 7?
  2. Sun Fortran 77: yes, if there are notes specific to the Sun fortran compiler, I think we can delete the entire section. The Sun compiler is long dead and we haven't tested with it for years -- we can just remove the entire content block about peculiarities of the Sun Fortran compiler.
  3. Move docs from deprecated into new functions: that would be wonderful. We should still definitely leave man pages for the old deprecated functions, but those docs can be short / abbreviated / just refer to the new/replacement functions.

jsquyres avatar Feb 08 '23 15:02 jsquyres

  1. Agreed, I'll have to think a bit here.. ;)
  2. Great! Lots of dead stuff can be removed!
  3. I'll make a draft PR so I can get feedback!

zerothi avatar Feb 08 '23 15:02 zerothi