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

Man page generation wrong in latest release #1049

Closed
dionysius opened this issue Mar 3, 2020 · 10 comments
Closed

Man page generation wrong in latest release #1049

dionysius opened this issue Mar 3, 2020 · 10 comments

Comments

@dionysius
Copy link

dionysius commented Mar 3, 2020

There's a difference between cobra 0.0.5 and 0.0.6 negatively affecting the resulting man files:

Example: https://play.golang.org/p/zcjf0eQ35xj (copy it locally to compare with v0.0.5)

diff --git a/tmp/man-from-005/prog.8 b/tmp/man-from-006/prog.8
index 166d0d1..afda3e3 100644
--- a/tmp/man-from-005/prog.8
+++ b/tmp/man-from-006/prog.8
@@ -1,7 +1,7 @@
-.TH "FOO" "8" "Mar 2020" "The Team" "System Manager's Manual" 
 .nh
-.ad l
-
+.TH FOO(8)Mar 2020
+The Team
+System Manager's Manual

The resulting visual representation is wrong: see man -l /tmp/man-from-006/prog.8.

While we're at it, can we maybe have a golden file/dir for GenMan and GenManTree to detect syntax changes?

I also get the following during execution locally, which might be related (when using GenManTree):

WARNING: go-md2man does not handle node type HTMLSpan
@github-actions
Copy link

github-actions bot commented May 30, 2020

This issue is being marked as stale due to a long period of inactivity

@MichaelEischer
Copy link

MichaelEischer commented Sep 21, 2020

I've bisected this to 77e4d5a (thanks for the example code), which updates md2man to v2.0.0 .

@dionysius
Copy link
Author

dionysius commented Oct 2, 2020

I'm still using v0.0.5 because of that...

@elboulangero
Copy link

elboulangero commented Oct 14, 2020

Seems like we are facing the same issue when while building the docker package in Debian. Since the go-md2man v2 transition, we can't build the documentation anymore:

dh_installman: warning: Section for ./.gopath/src/github.com/docker/cli/man/man1/docker-attach.1 is computed as "2020", which is not a valid section
dh_installman: error: Could not determine section for ./.gopath/src/github.com/docker/cli/man/man1/docker-attach.1
dh_installman: error: Aborting due to earlier error
make: *** [debian/rules:40: binary] Error 25

For the full bug report: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=971789

After some investigation, I observe the same changes in the man pages as the one showed above by @dionysius. And that's why dh_installman (the tool that checks the man pages while creating the Debian package) chokes, it's because something is wrong with the man page.

We solved that by patching doc/man_docs.go, as can be seen here: https://salsa.debian.org/docker-team/docker/-/blob/master/debian/patches/cli-fix-spf13-cobra-man-docs.patch

This patch applies to the lines:

cobra/doc/man_docs.go

Lines 148 to 152 in b97b5ea

buf.WriteString(fmt.Sprintf(`%% %s(%s)%s
%% %s
%% %s
# NAME
`, header.Title, header.Section, header.date, header.Source, header.Manual))

Note that I'm not familiar with cobra so I can't claim this is the right fix. However, one thing for sure is that the .TH field of a man page is supposed to have 2 mandatory fields + 3 optional fields. The fields can be surrounded by double-quotes in case a field contain a space. For reference: https://liw.fi/manpages/

@drakkan
Copy link

drakkan commented Oct 14, 2020

@elboulangero thank you. Do you mind opening a PR? Perhaps this way there is a better chance that the bug will be fixed

satta added a commit to satta/cobra that referenced this issue Oct 14, 2020
This addresses spf13#1049 by changing the format of the generated
Markdown input.
@MichaelEischer
Copy link

MichaelEischer commented Oct 14, 2020

I've noticed further changes in the generated manpages (to be precise: I've only looked at the generated manpages for restic). A CLI option used to look like this:

       -e, --exclude=[]
           exclude a pattern (can be specified multiple times)

Whereas now its

       -e, --exclude=[]      exclude a pattern (can be specified multiple times)

with a different indentation level for each option description, which looks really chaotic, e.g.

       --ignore-inode[=false]      ignore inode number changes when checking for modified files

@elboulangero
Copy link

elboulangero commented Oct 15, 2020

Do you mind opening a PR

PR opened at #1256

@satta
Copy link
Contributor

satta commented Oct 15, 2020

Do you mind opening a PR

PR opened at #1256

Ah, I already opened one for that (#1255)

@thaJeztah
Copy link
Contributor

thaJeztah commented Oct 17, 2020

Also opened cpuguy83/go-md2man#65 to make go-md2man accept the format (reviews welcome!)

drakkan pushed a commit to drakkan/cobra that referenced this issue Oct 17, 2020
This addresses spf13#1049 by changing the format of the generated
Markdown input.
jpmcb pushed a commit that referenced this issue Oct 18, 2020
This addresses #1049 by changing the format of the generated
Markdown input.
@jpmcb
Copy link
Collaborator

jpmcb commented Oct 18, 2020

Version 1.1.1 is live: https://github.com/spf13/cobra/releases/tag/v1.1.1

Feel free to re-open this issue if there are still problems!

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

No branches or pull requests

8 participants