lightning
lightning copied to clipboard
Remove mrkd
As requested by @wtogami ...
Changelog-None
Just a word of caution: lowdown
is only available in Ubuntu LTS starting from 22.04. So we might need to also add pointers to install it on older versions somehow.
Just a word of caution: lowdown is only available in Ubuntu LTS starting from 22.04. So we might need to also add pointers to install it on older versions somehow.
It isn't available on Fedora at all yet but I will newly package it and all other build deps for that distro to handle offline builds without pip or poetry.
It isn't available on Fedora at all yet but I will newly package it and all other build deps for that distro to handle offline builds without pip or poetry.
Mh! we really achieve the offline build by kicking off pip and poetry here? or are we just solve it for now? if rust enters as required dependencies (it already is a required dep because you can not change an RPC command without it) the offline problem is open again right? or there is some way to use cargo in an offline mode that I'm not aware of?
I'm more than a pro to kick off mrkd
because the maintainer is slow and unresponsive, and we can always put lowdown
as a submodule because we already build C, so should be not a big deal! (and we can always keep a fork if some bug exists, I think we have enough people with good knowledge of C to keep a tiny markdown parser alive :) )
Mh! we really achieve the offline build by kicking off pip and poetry here? or are we just solve it for now? if rust enters as required dependencies (it already is a required dep because you can not change an RPC command without it) the offline problem is open again right? or there is some way to use cargo in an offline mode that I'm not aware of?
Distros had long packaged python modules to such a degree that many users never have used pip
.
https://doc.rust-lang.org/cargo/commands/cargo-vendor.html
The usual solution for offline rust builds is with cargo vendor
.
I'm more than a pro to kick off
mrkd
because the maintainer is slow and unresponsive, and we can always putlowdown
as a submodule because we already build C, so should be not a big deal! (and we can always keep a fork if some bug exists, I think we have enough people with good knowledge of C to keep a tiny markdown parser alive :) )
lowdown
can easily be a tool packaged in distros. Building and running it as a build dependency is less desirable for reasons like it wouldn't work for cross-compiles.
Manually inspected diff -u lightning-before/doc lightning-after/doc
and found some oddities.
lightning-decodepay.7
Before
After
I prefer the more information dense bullets. But it mangled min_final_cltv_expiry
in such a way that man misinterprets some _
literals as underline.
Here's the diff -u
of particularly the min_final_cltv_expiry
example from above.
These before and after shots show the typical difference from mrkd to lowdown.
Before
After
- What bothers me about many of the lowdown formatted outputs is the 1st level bullets are not indented.
- Various escaping changes make almost no difference except when it causes errors like in the above
decodepay
example. This page had no problems just theSEE ALSO
is no longer bold.
.gitignore
# Ignore generated files
devtools/features
doc/lightning*.[1578]
I ran into a confusing issue when switching between before and after branches where dependency tracking failed to recognize it needed to re-process the man pages. I then tried make clean
and it still failed to redo these files. Perhaps make clean
could also additionally delete the same files ignored by .gitignore
?
Here's the diff -u
of the doc/
directory generated before and after this PR.
https://fedorapeople.org/~wtogami/a/2022/e70729b04befe44d603370b96fbb10dc00bd342d...77f0388b6757bc6c7b9909521b0eadb0c55bd773-manpage.diff
~~Comparing before and after is tedious but we should also visually inspect the HTML formatted output as a sanity check so it won't be a surprise later when the docs website is regenerated.~~ edit: Oops we use sphinx to generate the documentation web pages. So we use lowdown for only man page generation? Then there isn't much remaining to inspect.
For the record, clightning is now broken in nixpkgs and nix-bitcoin because mrkd is now broken in nixpkgs - https://github.com/NixOS/nixpkgs/blob/0b73b43a942d4f264036c74d03758194ba0b5855/pkgs/development/python-modules/mrkd/default.nix#L27-L28
The above review found only cosmetic or minor formatting bugs. Build being broken is a bigger problem. For that reason maybe we should merge this before 0.12?
I don't think this should go in now, at it risks breaking every other platform that now needs to figure out how to get lowdown, update their docs etc.
How about pre-process the man pages, commit and skip that build step for the 0.12 release? That would be the least risky change while maximizing ease of deployment. Revert it immediately after 0.12 is cut.
Here's the
diff -u
of particularly themin_final_cltv_expiry
example from above.
OK, I've fixed this now.
Rebased, and formatting fix. The extra blank line before sub-indents is a bit jarring, but seems the best we can do for now?
(See man -l doc/lightning-decode.7)
Just a word of caution:
lowdown
is only available in Ubuntu LTS starting from 22.04. So we might need to also add pointers to install it on older versions somehow.
Latest draft here will build one internally if it's not found by configure. It seems fairly portable...