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

Add ManSection enum, own title line struct and other minor updates #14

Closed
wants to merge 14 commits into from
Closed
49 changes: 49 additions & 0 deletions .github/workflows/pipeline.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
name: pipeline

on: [push, pull_request]

jobs:
check:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2

- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true
components: rustfmt, clippy

- uses: Swatinem/rust-cache@v1

- name: build
uses: actions-rs/cargo@v1
with:
command: test
args: --no-run

- name: test
uses: actions-rs/cargo@v1
with:
command: test
args: -- --nocapture --quiet

- name: examples
uses: actions-rs/cargo@v1
with:
command: test
args: --examples -- --nocapture --quiet

- name: formatting
uses: actions-rs/cargo@v1
with:
command: fmt
args: --all -- --check

- name: check
uses: actions-rs/cargo@v1
with:
command: check

16 changes: 15 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,16 @@
target
### Created by https://www.gitignore.io
### Rust ###
# Generated by Cargo
# will have compiled files and executables
debug/
target/

# Remove Cargo.lock from gitignore if creating an executable, leave it for libraries
# More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html
Cargo.lock

# These are backup files generated by rustfmt
**/*.rs.bk

# MSVC Windows builds of rustc generate these, which store debugging information
*.pdb
Comment on lines +1 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

How come you are changing the .gitignore?

Copy link
Author

Choose a reason for hiding this comment

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

Force of habit, it carried over from my previous work on roff-rs, I can revert this if you want to.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please do so. imo person/machine specific ignores should be set in the personal ignores rather than checked into the shared ignores.

4 changes: 0 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,3 @@ pre-release-replacements = [
]

[dependencies]

[dev-dependencies]
pretty_assertions = "1.0.0"
duct = "0.13"
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
# roff-rs

[![Build Status](https://github.com/rust-cli/roff-rs/workflows/pipeline/badge.svg)][CI]
[![Documentation](https://img.shields.io/badge/docs-master-blue.svg)][Documentation]
![License](https://img.shields.io/crates/l/roff.svg)
[![crates.io](https://img.shields.io/crates/v/roff.svg)][Crates.io]

[Crates.io]: https://crates.io/crates/roff
[Documentation]: https://docs.rs/roff/

[Roff](http://man7.org/linux/man-pages/man7/roff.7.html) generation library.

## Examples
Expand Down Expand Up @@ -98,3 +96,7 @@ Unless you explicitly state otherwise, any contribution intentionally
submitted for inclusion in the work by you, as defined in the Apache-2.0
license, shall be dual licensed as above, without any additional terms or
conditions.

[CI]: https://github.com/rust-cli/roff-rs/actions
[Crates.io]: https://crates.io/crates/roff
[Documentation]: https://docs.rs/roff/
57 changes: 57 additions & 0 deletions examples/demo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
use roff::*;

// View this example by running `cargo run --example demo | man -l -`.

fn main() {
let page = Roff::new("corrupt", ManSection::Executable)
.date("2021-12-25")
.manual("General Commands Manual")
.source("corrupt v1")
.section(
"name",
&["corrupt - modify files by randomly changing bits"],
)
.section(
"SYNOPSIS",
&[
bold("corrupt"),
" ".into(),
"[".into(),
bold("-n"),
" ".into(),
italic("BITS"),
"]".into(),
" ".into(),
"[".into(),
bold("--bits"),
" ".into(),
italic("BITS"),
"]".into(),
" ".into(),
italic("file"),
"...".into(),
],
)
.section(
"description",
&[
bold("corrupt"),
" modifies files by toggling a randomly chosen bit.".into(),
],
)
.section(
"options",
&[list(
&[
bold("-n"),
", ".into(),
bold("--bits"),
"=".into(),
italic("BITS"),
],
&["Set the number of bits to modify. ", "Default is one bit."],
)],
);

println!("{}", page.render());
}
135 changes: 117 additions & 18 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,123 @@
use std::fmt::Write;

#[derive(PartialEq, Eq)]
pub struct Roff {
/// Title line for a manpage.
pub struct Title {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should at least implement Debug and Clone on everything

Copy link
Author

Choose a reason for hiding this comment

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

Good point!

title: String,
section: i8,
section: ManSection,
date: Option<String>,
source: Option<String>,
manual: Option<String>,
}
Comment on lines +4 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

How much of this is roff vs man policy?


impl Title {
pub fn new(title: &str, section: ManSection) -> Self {
Title {
title: title.into(),
section,
date: None,
source: None,
manual: None,
}
}
}

impl Troffable for Title {
fn render(&self) -> String {
let manual = self.manual.as_deref().unwrap_or_default();
let date = self.date.as_deref().unwrap_or_default();
let source = self.source.as_deref().unwrap_or_default();

format!(
r#".TH "{}" "{}" "{}" "{}" "{}""#,
self.title.to_uppercase(),
self.section.value(),
date,
source,
manual
)
}
}

/// Manpage sections.
///
/// The most common is [`ManSection::Executable`], and is the recommended default.
#[derive(Clone, Copy)]
pub enum ManSection {
/// Executable programs or shell commands
Executable,
/// System calls (functions provided by the kernel)
SystemCalls,
/// Library calls (functions within program libraries)
LibraryCalls,
/// Special files (usually found in /dev)
SpecialFiles,
/// File formats and conventions, e.g. /etc/passwd
FileFormats,
/// Games
Games,
/// Miscellaneous (including macro packages and conventions), e.g. man(7), groff(7)
Miscellaneous,
/// System administration commands (usually only for root)
SystemAdministrationCommands,
/// Kernel routines [Non standard]
KernelRoutines,
}

Comment on lines +41 to +65
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm missing something, this seems like man policy when this crate is focused on the underlying markup.

Copy link
Contributor

Choose a reason for hiding this comment

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

Your two comments about whether things should go here or in a more manpage specific crate was one of the reasons I ended up trying to do everything in clap_man, the inclusion of a title line means we're already targeting manpages and not "pure" roff. It is a massive type-setting system (here is short reference of GNU variant of the format, you can see we're only really touching a tiny, tiny part of it), and has historically been used for lots of different things from books, articles and more.

Nowadays however I thing nobody uses it outside of manpages anymore, and while the man crate contains useful tools to build manpages for CLI tools this crate already targets a subset of roff specific to manpages, at least in my opinion. sweat_smile roffman that you mentioned in the PR in clap has sections in it, though the name implies that it is specific for manpages, but not a huge fan of its API.

Sorry, this is a lot of words for saying that in my head roff-rs is already a manpage specific crate slightly_smiling_face It's again one of the reasons I've attempted a few different approaches to this, I haven't found a good solution for it yet besides creating my own crate for this.

My thought is to prefer to keep this crate man agnostic unless its getting in the way of getting clap_man done.

It looks like we could have the Section enum and the title formatting external to this crate and lose nothing but I'm not as knee deep in both crates as you are, so I defer to your judgement.

Copy link
Author

Choose a reason for hiding this comment

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

Due to the current architecture this crate cannot be used to create anything besides manpages, so it is in a sense already specific to manpages and not agnostic. I'm starting to lean more towards having a more intermediate crate specifically for creating manpages, I have a prototype laying around that I could revive. Thoughts?

impl ManSection {
pub fn value(&self) -> i8 {
match self {
ManSection::Executable => 1,
ManSection::SystemCalls => 2,
ManSection::LibraryCalls => 3,
ManSection::SpecialFiles => 4,
ManSection::FileFormats => 5,
ManSection::Games => 6,
ManSection::Miscellaneous => 7,
ManSection::SystemAdministrationCommands => 8,
ManSection::KernelRoutines => 9,
}
}
}

pub struct Roff {
Copy link
Contributor

Choose a reason for hiding this comment

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

If Roff is the entry point for this crate. please keep it first in the file so its easier to jump in and see what this crate does

Copy link
Author

Choose a reason for hiding this comment

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

Will fix!

title: Title,
content: Vec<Section>,
}

impl Roff {
pub fn new(title: &str, section: i8) -> Self {
pub fn new(title: &str, section: ManSection) -> Self {
Roff {
title: title.into(),
section,
title: Title::new(title, section),
content: Vec::new(),
}
}

/// Date of the last nontrivial change to the manpage. Should be formatted
/// in `YYYY-MM-DD`.
pub fn date(mut self, date: impl Into<String>) -> Self {
self.title.date = Some(date.into());
self
}
Comment on lines +95 to +100
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we accept year, month, day arguments so we can enforce formatting or eventually add a date/time lib dependency (haven't looked to see where the community is at with time vs chrono)

Copy link
Author

Choose a reason for hiding this comment

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

That's probably a better way of doing it, good point. Not sure I'd like to pull in a full library for date changes here, we might do it in clap_man instead to fill these in instead.


/// The source of the command, function or system call.
pub fn source(mut self, source: impl Into<String>) -> Self {
self.title.source = Some(source.into());
self
}

/// The title of the manual.
pub fn manual(mut self, manual: impl Into<String>) -> Self {
self.title.manual = Some(manual.into());
self
}

pub fn section<'a, C, I>(mut self, title: &str, content: I) -> Self
where
I: IntoIterator<Item = &'a C>,
C: Troffable + 'a,
{
let title = title.into();
let content = content.into_iter().map(|x| x.render()).collect();
let content = content.into_iter().map(Troffable::render).collect();

self.content.push(Section { title, content });
self
Expand All @@ -33,13 +128,18 @@ impl Troffable for Roff {
fn render(&self) -> String {
let mut res = String::new();

writeln!(
&mut res,
".TH {} {}",
self.title.to_uppercase(),
self.section
)
.unwrap();
writeln!(&mut res, "{}", self.title.render()).unwrap();

// Compatibility settings:
Copy link

@larswirzenius larswirzenius Dec 20, 2021

Choose a reason for hiding this comment

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

Why is this needed? What requires turning of justification and hyphenation?

I'm looking at this from a manual page generation point of view, and non-justified manual pages are unusual, and thus jarring to read. I could live with it, but I'd like the comment to explain what requires it.

Copy link
Contributor

Choose a reason for hiding this comment

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

On the topic of compat settings, asciidoctor does https://github.com/asciidoctor/asciidoctor/blob/main/lib/asciidoctor/converter/manpage.rb#L61 (with links out for information)

Choose a reason for hiding this comment

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

The point about plain apostrophes is a good one, and I should add that to my roff crate version. However, I can't see any explanation of why hyphenation and justification should be turned off. The sentence space size I can live with: it removes the need to make sure sentences end with a newline after the terminating punctuation.

//
// Set sentence_space_size to 0 to prevent extra space between sentences separated
// by a newline the alternative is to add \& at the end of the line
writeln!(&mut res, ".ss \\n[.ss] 0").unwrap();
// Disable hyphenation
writeln!(&mut res, ".nh").unwrap();
// Disable justification (adjust text to the left margin only)
writeln!(&mut res, ".ad l").unwrap();

epage marked this conversation as resolved.
Show resolved Hide resolved
for section in &self.content {
writeln!(&mut res, "{}", escape(&section.render())).unwrap();
}
Expand All @@ -48,7 +148,6 @@ impl Troffable for Roff {
}
}

#[derive(PartialEq, Eq)]
Copy link
Contributor

Choose a reason for hiding this comment

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

How come you've been dropping PartialEq, Eq from these items?

Copy link
Author

Choose a reason for hiding this comment

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

I don't really know why it's derived, I can't imagine it was used for something besides testing (the first full commit has them derived but not used) because comparing manpages for equality sounds very niche.

Copy link
Contributor

Choose a reason for hiding this comment

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

I lean towards keeping them (slightly less important than Clone and Debug) though we should make sure we call this out as a breaking change.

struct Section {
title: String,
content: String,
Expand All @@ -58,7 +157,7 @@ impl Troffable for Section {
fn render(&self) -> String {
let mut res = String::new();

writeln!(&mut res, ".SH {}", self.title.to_uppercase()).unwrap();
writeln!(&mut res, ".SH \"{}\"", self.title.to_uppercase()).unwrap();

Choose a reason for hiding this comment

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

This will fail if the title contains quote characters. I'm not sure how to fix that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oddly, I'm not seeing any quote escaping in asciidoctor, only left/right quotes. I wonder if that is an artifact of them only using left/right quotes

https://github.com/asciidoctor/asciidoctor/blob/main/lib/asciidoctor/converter/manpage.rb#L716

Copy link
Contributor

Choose a reason for hiding this comment

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

For some more information on escaping, check out https://www.ibm.com/docs/en/aix/7.2?topic=t-troff-command (search for mm Strings)

Choose a reason for hiding this comment

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

I've been reading the groff(7), and I can't see any way to escape quotes in control lines or macro arguments.

res.push_str(&self.content);

res
Expand Down Expand Up @@ -101,10 +200,10 @@ pub fn italic(input: &str) -> String {
format!(r"\fI{}\fP", input)
}

pub fn list<C1: Troffable, C2: Troffable>(header: &'_ [C1], content: &'_ [C2]) -> String {
pub fn list<C1: Troffable, C2: Troffable>(header: &[C1], content: &[C2]) -> String {
format!(".TP\n{}\n{}", header.render(), content.render())
}

fn escape(input: &str) -> String {
pub fn escape(input: &str) -> String {
input.replace("-", r"\-")
}
Comment on lines +207 to 209
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is escape public? It seems like the API should be enforcing whats escaped or not

Copy link
Author

Choose a reason for hiding this comment

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

This was an oversight on my part, I forgot that everything is escaped when rendered, I'll fix this.

36 changes: 23 additions & 13 deletions tests/demo.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
extern crate duct;
extern crate roff;
#[macro_use]
extern crate pretty_assertions;
use std::{
io::Write,
process::{Command, Stdio},
};

use roff::*;

fn roff_to_ascii(input: &str) -> String {
duct::cmd("troff", &["-a", "-mman"])
.stdin_bytes(input)
.stdout_capture()
.read()
.unwrap()
let mut cmd = Command::new("troff")
.args(["-a", "-mman"])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()
.unwrap();

if let Some(ref mut stdin) = cmd.stdin {
stdin.write_all(input.as_bytes()).unwrap();
}

String::from_utf8(cmd.wait_with_output().unwrap().stdout).unwrap()
}

#[test]
fn demo() {
use roff::*;

let page = Roff::new("corrupt", 1)
let page = Roff::new("corrupt", ManSection::Executable)
.date("2021-12-25")
.manual("General Commands Manual")
.source("corrupt v1")
.section(
"name",
&["corrupt - modify files by randomly changing bits"],
Expand Down Expand Up @@ -64,7 +74,7 @@ fn demo() {

// use std::io::Write;
// let mut f = ::std::fs::File::create("./tests/demo.generated.troff").unwrap();
// f.write_all(&page.render().as_bytes());
// f.write_all(&page.render().as_bytes()).unwrap();

assert_eq!(
roff_to_ascii(include_str!("./demo.troff")),
Expand Down
Loading