-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
Restore man page rendering #202
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
/ansi2html.1 | ||
/ansi2html.1.xml |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include this as part of normal testing in
[testenv]
section to see what happens on macos. Based on the couple of hours I spent trying to get a2x to work on macos, I realised that is kinda broken and that nobody will bother to add required fixes.As that man page is minimalistic we should find a solution to build it that can run on all platforms supported by ansi2html tool.
PS. Installing docbook on macos in to the problem, it does install just fine with brew. The problem is that the build fails somewhere with xml where it tries to access the network for a DTD but docbook is configured to run the command with network access disabled, than apparently that was hardcoded. On linux it appears to have a local database which allows it to run on offline mode, something that is not part of the homebrew install. It looked as a total mess, and I am sure that we can find a better maintained replacement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ssbarnea what is the error output of a2x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not keen to let this go in, here is the proof that it breaks at least
tox -e pkg
on macos:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It adds too many non-python dependencies for even building the package. That is not cool or portable.
Officially there is no way to install man pages when installing python packages so I am inclined to say that maybe the man page should not be part of the wheel distribution.
This is not the first project where I encounter this issue. "man pages not really compatible with pip". Still, I do not know what would be the optimal way to deal with this conundrum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ssbarnea all of these dependencies are portable and can be installed via homebrew, correct? Also: Is there need for
tox -e pkg
to work on macOS if we only ever do releases from Linux? The end user does not even touch tox, only contributors do, and access to Linux is easy on macOS using Docker. The alternative to the current approach is that (a) all distros have to build the man page at packaging time and install those build dependencies that they do not need right now or (b) we keep a copy of that file in Git and add CI that goes red whenever they go out of sync by rendering the man page and comparing it to the version in Git. But we're fixing a regression here. Let's first fix the regression and then discuss alternatives, things are in an unreleasable state for too long already, and improvement and fixing should not depend on each other. Thanks.