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

sondr3 · 2021-12-18T14:05:29Z

The title line, manpage section and compatibility settings come from clap-rs/clap#3174, where we decided on extracting out Roff-specific functionality to this crate. I also did a few minor changes, like removing the derive for PartialEq and Eq from the structs. The title line struct will make #7 obsolete, and is in my opinion a better solution as the order of the elements is important and is not just a list of strings.

cc @epage

epage · 2021-12-18T18:58:05Z

.gitignore

+### 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


How come you are changing the .gitignore?

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

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

epage · 2021-12-18T18:59:38Z

src/lib.rs

+/// 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,
+}
+


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

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.

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?

epage · 2021-12-18T19:00:53Z

src/lib.rs

+    /// 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
+    }


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)

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.

epage · 2021-12-18T19:01:25Z

src/lib.rs

+    }
+}
+
+pub struct Roff {


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

epage · 2021-12-18T19:02:05Z

src/lib.rs

+pub struct Title {
    title: String,
-    section: i8,
+    section: ManSection,
+    date: Option<String>,
+    source: Option<String>,
+    manual: Option<String>,
+}


How much of this is roff vs man policy?

src/lib.rs

epage · 2021-12-18T19:03:50Z

src/lib.rs

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


We should at least implement Debug and Clone on everything

Good point!

epage · 2021-12-18T19:04:28Z

src/lib.rs

@@ -48,7 +148,6 @@ impl Troffable for Roff {
    }
 }

-#[derive(PartialEq, Eq)]


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

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.

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.

epage · 2021-12-18T19:05:41Z

src/lib.rs

+pub fn escape(input: &str) -> String {
    input.replace("-", r"\-")
 }


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

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

sondr3 · 2021-12-18T19:48:23Z

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. 😅 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 🙂 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.

larswirzenius · 2021-12-20T08:00:37Z

src/lib.rs

@@ -109,7 +109,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();


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

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

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)

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

larswirzenius · 2021-12-20T08:26:09Z

src/lib.rs

@@ -130,6 +130,16 @@ impl Troffable for Roff {

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

+        // Compatibility settings:


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.

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)

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.

larswirzenius · 2021-12-20T18:06:26Z

I had a closer look at the manual page generation and the roff crate. I've pushed two branches that I think would be an improvement.

https://github.com/larswirzenius/roff-rs/tree/liw/roff-only changes roff to have an abstraction level of ROFF only, dropping manual page stuff. The commit message has some more detailed explanations. The branch isn't ready to be merged, and needs some cleanup first.

I then ported @sondr3's maual page generation to my roff version (https://github.com/larswirzenius/clap/tree/liw/generate-man). This, too, probably needs cleanup.

What do you think? Is my approach of any interest?

epage · 2021-12-20T18:26:45Z

What do you think? Is my approach of any interest?

I really like this! By focusing on the roff structure, I feel like it makes clearer the intent of what is going on while giving the benefit of escaping and typing (compared to doing everything with raw strings).

sondr3 · 2021-12-21T11:47:13Z

I too like your approach and from the comment over on the PR to clap you have far more experience working with manpages and the roff-format, so you can be the judge for this crate. Do you want to create a PR against my fork of clap so we can integrate your changes once you feel it's ready? 😄

larswirzenius · 2021-12-21T12:11:07Z

I will do that later today or tomorrow, as soon as I've dug myself out from this horde of elves I'm buried under.

sondr3 · 2021-12-23T14:53:37Z

Superseded by #15.

sondr3 added 14 commits December 18, 2021 14:56

feat: Create Title struct for title line

8ff1a40

fix: Wrap section titles in quotes

0fb32f2

fix: Update demo.troff to match new output

d485bbf

fix: Clippy lints

5d928a4

chore: Move to GitHub Actions for CI

749f443

chore: Create example

c300e98

feat: Create enum for man page sections

19d169a

refactor: Remove uneeded PartialEq, Eq derives

251d935

chore: Remove devdeps, update .gitignore

3d3515f

fix: Make escape function public

6bb464d

feat: Make ManSection derive Clone, Copy

9a5ca7f

fix: Add some compatibility settings to renderer

d591ca8

feat: Add paragraph function

d4a176a

chore: Refactor CI

5401897

sondr3 mentioned this pull request Dec 18, 2021

WIP: Man page generation clap-rs/clap#3174

Merged

4 tasks

epage reviewed Dec 18, 2021

View reviewed changes

src/lib.rs Show resolved Hide resolved

epage reviewed Dec 18, 2021

View reviewed changes

larswirzenius reviewed Dec 20, 2021

View reviewed changes

sondr3 closed this Dec 23, 2021

sondr3 deleted the updates branch June 17, 2024 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

sondr3 commented Dec 18, 2021 •

edited

Loading

epage Dec 18, 2021

sondr3 Dec 18, 2021

epage Dec 18, 2021

epage Dec 18, 2021

epage Dec 19, 2021

sondr3 Dec 19, 2021

epage Dec 18, 2021

sondr3 Dec 18, 2021

epage Dec 18, 2021

sondr3 Dec 18, 2021

epage Dec 18, 2021

epage Dec 18, 2021

sondr3 Dec 18, 2021

epage Dec 18, 2021

sondr3 Dec 18, 2021

epage Dec 18, 2021

epage Dec 18, 2021

sondr3 Dec 18, 2021

sondr3 commented Dec 18, 2021 •

edited

Loading

larswirzenius Dec 20, 2021

epage Dec 20, 2021

epage Dec 20, 2021

larswirzenius Dec 20, 2021

larswirzenius Dec 20, 2021 •

edited

Loading

epage Dec 20, 2021

larswirzenius Dec 21, 2021

larswirzenius commented Dec 20, 2021

epage commented Dec 20, 2021

sondr3 commented Dec 21, 2021 •

edited

Loading

larswirzenius commented Dec 21, 2021

sondr3 commented Dec 23, 2021

		@@ -130,6 +130,16 @@ impl Troffable for Roff {

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

		// Compatibility settings:

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

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

Conversation

sondr3 commented Dec 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sondr3 commented Dec 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larswirzenius Dec 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larswirzenius commented Dec 20, 2021

epage commented Dec 20, 2021

sondr3 commented Dec 21, 2021 • edited Loading

larswirzenius commented Dec 21, 2021

sondr3 commented Dec 23, 2021

sondr3 commented Dec 18, 2021 •

edited

Loading

sondr3 commented Dec 18, 2021 •

edited

Loading

larswirzenius Dec 20, 2021 •

edited

Loading

sondr3 commented Dec 21, 2021 •

edited

Loading