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

Process line breaks in description #128

Open
theli-ua opened this issue Jun 7, 2022 · 5 comments
Open

Process line breaks in description #128

theli-ua opened this issue Jun 7, 2022 · 5 comments

Comments

@theli-ua
Copy link
Contributor

theli-ua commented Jun 7, 2022

Unless I'm missing something it is incredibly painfull to format descriptions with multiple lines. eg

/// Multiple choice option.
///    Possible values:
///         * foo - FOO
///         * bar - BAR
@Timmmm
Copy link

Timmmm commented Jul 14, 2022

Same as #121. :-/

@Timmmm
Copy link

Timmmm commented Jul 14, 2022

I had a little look into the code. I think there are two issues:

  1. The docstring is concatenated into one big string ignoring newlines. See this code:

    syn::LitStr::new(&(previous.value() + &*lit_str.value()), previous_span)

  2. The printing code doesn't work quite right even if you do have newlines in the input. See this code.

Here is that code extracted with some example input.

The second issue is easy to solve. The first is more difficult because it isn't obvious when you want newlines. E.g. in this case you clearly do:

Can be one of
* A
* B
* C

But here you probably don't

There are many things
this can be including A,
B and C.

I vaguely recall some other argument parse "solving" this by letting you put {n} for a newline, but I'm not sure I really like that. I think maybe we could get something good with one simple trick.

  1. Any line that starts with [A-Za-z] is appended to the previous line, unless the end of the output is currently \n\n (i.e. a blank line).

I think that would correctly format something like this:

This bit is prose and should
be wrapped into a paragraph.

But this is another paragraph so there
should be a new line between
them.

1. A list would work as expected.
3. Even with long
   entries like this though they
   won't be wrapped nicely.

| Tables | Should |
|------|------|
| Also be | Fine |

I'll try it out.

@Timmmm
Copy link

Timmmm commented Jul 14, 2022

Ok the logic turned out to be slightly more complicated than I thought, but this seems to work:

fn parse_comment(lines: &[&str]) -> String {
    let mut out = String::new();
    for line in lines {
        let line_starts_with_letter = line.bytes().next().map(|c| c.is_ascii_alphabetic()).unwrap_or(false);
        if line_starts_with_letter {
            if !out.is_empty() {
                if out.ends_with('\n') {
                    out.push('\n');
                } else {
                    out.push(' ');
                }
            }
        } else {
            out.push('\n');
        }
        
        out.push_str(line);
    }
    out
}

fn main() {
    let comment = "This bit is prose and should
be wrapped into a paragraph.

But this is another paragraph so there
should be a new line between
them.

1. A list would work as expected.
3. Even with long
   entries like this though they
   won't be wrapped nicely.

| Tables | Should |
|------|------|
| Also be | Fine |";

    let description = parse_comment(&comment.lines().collect::<Vec<_>>());
    
    println!("{}", description);
}

Output:

This bit is prose and should be wrapped into a paragraph.

But this is another paragraph so there should be a new line between them.

1. A list would work as expected.
3. Even with long
   entries like this though they
   won't be wrapped nicely.

| Tables | Should |
|------|------|
| Also be | Fine |

Note the paragraphs are joined together into one line but the other bits aren't.

Now just to fix the printing code...

@Timmmm
Copy link

Timmmm commented Jul 14, 2022

Ok that turned out to be easy - just loop through the lines in the description:

    for line in cmd.description.lines() {
        let mut words = line.split(' ').peekable();
        while let Some(first_word) = words.next() {
            indent_description(&mut current_line);
            current_line.push_str(first_word);
    
            while let Some(&word) = words.peek() {
                if (char_len(&current_line) + char_len(word) + 1) > WRAP_WIDTH {
                    new_line(&mut current_line, out);
                    break;
                } else {
                    // advance the iterator
                    let _ = words.next();
                    current_line.push(' ');
                    current_line.push_str(word);
                }
            }
        } 
        new_line(&mut current_line, out);
    }

Output is:

  target            This bit is prose and should be wrapped into a paragraph.
                    
                    But this is another paragraph so there should be a new line
                    between them.
                    
                    1. A list would work as expected.
                    3. Even with long
                       entries like this though they
                       won't be wrapped nicely.
                    
                    | Tables | Should |
                    |------|------|
                    | Also be | Fine |

Note the first two paragraphs have been reflowed but everything else is left as is.

Full demo here.

I will prepare a PR if somebody from the project says they would accept it.

Timmmm added a commit to Timmmm/argh that referenced this issue Jul 15, 2022
@Timmmm
Copy link

Timmmm commented Jul 15, 2022

Actually I went ahead and did it because I can use it without waiting for a PR to be accepted. Just add this to your Cargo.toml:

[patch.crates-io]
argh = { git = "https://github.com/Timmmm/argh", branch = "multiline_docs" }

Timmmm added a commit to Timmmm/argh that referenced this issue Jul 15, 2022
Timmmm added a commit to Timmmm/argh that referenced this issue Nov 16, 2023
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

Successfully merging a pull request may close this issue.

2 participants