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

Final word in help text is not wrapped #828

Closed
mgeisler opened this issue Jan 29, 2017 · 2 comments
Closed

Final word in help text is not wrapped #828

mgeisler opened this issue Jan 29, 2017 · 2 comments
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies
Milestone

Comments

@mgeisler
Copy link
Contributor

mgeisler commented Jan 29, 2017

Affected Version of clap

Tested with latest master, d29d7cc.

Expected Behavior Summary

The final word in the help text should be wrapped.

Actual Behavior Summary

The final word remain with the next to final word, that is, the last space is ignored.

Steps to Reproduce the issue

#[test]
fn final_word_wrapping() {
    let app = App::new("ctest").version("0.1").set_term_width(24);
    assert!(test::compare_output(app, "ctest --help", FINAL_WORD_WRAPPING, false));
}

static FINAL_WORD_WRAPPING: &'static str = "ctest 0.1

USAGE:
    ctest

FLAGS:
    -h, --help
            Prints help
            information
    -V, --version
            Prints
            version information
";

The "version information" line is longer than 12 characters, despite information being only 11 characters wide.

I have a fix for this that I'll post next.

@kbknapp
Copy link
Member

kbknapp commented Jan 30, 2017

Interesting, I wonder if it's counting the space? Could you post a gist of the output when you compile clap with features = ["debug"] and run the test program?

@kbknapp kbknapp added A-help Area: documentation, including docs.rs, readme, examples, etc... D: intermediate C-bug Category: Updating dependencies labels Jan 30, 2017
@kbknapp kbknapp added this to the 2.20.2 milestone Jan 30, 2017
mgeisler added a commit to mgeisler/clap-rs that referenced this issue Jan 30, 2017
mgeisler added a commit to mgeisler/clap-rs that referenced this issue Jan 30, 2017
Before, inserting a newline did not move the prev_space index forward.
This meant that the next word was measured incorrectly since the
length was measured back to the word before the newly inserted
linebreak.

Fixes clap-rs#828.
@mgeisler
Copy link
Contributor Author

I've put up a PR for fixing this and a small related bug. As explained, I hope to do a drop-in replacement later with my textwrap crate, but wanted to make sure the output didn't change first. So I compared the existing code with the wrapping done by that crate and found small problems.

mgeisler added a commit to mgeisler/clap-rs that referenced this issue Jan 30, 2017
Before, inserting a newline did not move the prev_space index forward.
This meant that the next word was measured incorrectly since the
length was measured back to the word before the newly inserted
linebreak.

Fixes clap-rs#828.
mgeisler added a commit to mgeisler/clap-rs that referenced this issue Jan 30, 2017
homu added a commit that referenced this issue Jan 30, 2017
Fix wrapping bugs

I've been working towards integrating my [textwrap][1] crate and along the way, I found some small problems in the existing `wrap_help` function in clap. I basically added new code that calls both `textwrap::fill` and `wrap_help` and panicked on any difference:
```
fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
    let input = help.clone();
    let mut old = help.clone();
    old_wrap_help(&mut old, longest_w, avail_chars);

    let mut wrapped = String::with_capacity(help.len());
    for (i, line) in help.lines().enumerate() {
        if i > 0 {
            wrapped.push('\n');
        }
        wrapped.push_str(&textwrap::fill(line, avail_chars));
    }

    // TODO: move up, This keeps old behavior of not wrapping at all
    // if one of the words would overflow the line
    if longest_w < avail_chars {
        *help = wrapped;
    }

    if *old != *help {
        println!("********************************");
        println!("longest_w: {}, avail_chars: {}", longest_w, avail_chars);
        println!("help: {:3} bytes: {:?}", input.len(), input);
        println!("old:  {:3} bytes: {:?}", old.len(), old);
        println!("new:  {:3} bytes: {:?}", help.len(), help);
        println!("********************************");
        panic!("bad wrap");
    }
}

fn old_wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
    // ... as before
```

This PR fixes two small problems discovered this way, one of which became #828.

[1]: https://crates.io/crates/textwrap
homu added a commit that referenced this issue Jan 30, 2017
Fix wrapping bugs

I've been working towards integrating my [textwrap][1] crate and along the way, I found some small problems in the existing `wrap_help` function in clap. I basically added new code that calls both `textwrap::fill` and `wrap_help` and panicked on any difference:
```
fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
    let input = help.clone();
    let mut old = help.clone();
    old_wrap_help(&mut old, longest_w, avail_chars);

    let mut wrapped = String::with_capacity(help.len());
    for (i, line) in help.lines().enumerate() {
        if i > 0 {
            wrapped.push('\n');
        }
        wrapped.push_str(&textwrap::fill(line, avail_chars));
    }

    // TODO: move up, This keeps old behavior of not wrapping at all
    // if one of the words would overflow the line
    if longest_w < avail_chars {
        *help = wrapped;
    }

    if *old != *help {
        println!("********************************");
        println!("longest_w: {}, avail_chars: {}", longest_w, avail_chars);
        println!("help: {:3} bytes: {:?}", input.len(), input);
        println!("old:  {:3} bytes: {:?}", old.len(), old);
        println!("new:  {:3} bytes: {:?}", help.len(), help);
        println!("********************************");
        panic!("bad wrap");
    }
}

fn old_wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
    // ... as before
```

This PR fixes two small problems discovered this way, one of which became #828.

[1]: https://crates.io/crates/textwrap
@homu homu closed this as completed in 564c5f0 Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-help Area: documentation, including docs.rs, readme, examples, etc... C-bug Category: Updating dependencies
Projects
None yet
Development

No branches or pull requests

2 participants