Fix wrapping bugs#832

Merged

homu merged 4 commits intoclap-rs:masterfrom

mgeisler:fix-wrapping-bugs

Jan 31, 2017

Contributor

mgeisler commented Jan 30, 2017

I've been working towards integrating my textwrap 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.

coveralls commented Jan 30, 2017 •

edited

Loading

Coverage increased (+0.006%) to 89.088% when pulling 08be630 on mgeisler:fix-wrapping-bugs into 72d7bf3 on kbknapp:master.

Member

kbknapp commented Jan 30, 2017

Thanks for tackling this! I'm fine with the coveralls failing, so don't worry about that. But I need the Travis checks to pass, and it looks like it's failing on stable and 1.11.0 (minimum supported version of Rust).

kbknapp added the R: work in progress label

Contributor Author

mgeisler commented Jan 30, 2017

Yeah, I'll look into the build failures later -- hopefully tonight!

mgeisler force-pushed the fix-wrapping-bugs branch from 08be630 to 78dec94 Compare

January 30, 2017 17:32

mgeisler added 4 commits

January 30, 2017 18:33


          fix: include final character in line lenght

aff4ba1

Before, wrapping the help text at, say, 80 characters really meant
that every line could be at most 79 characters wide.

Lines can now be up to and including avail_chars columns wide.

If needed, a desired margin or padding can be subtracted from the
avail_chars argument at a later point.


          fix: actually show character in debug output

84d8c54

The &help[j..j] string slice was empty so nothing was shown.


          tests: show how final word is not always wrapped

563a539

This is clap-rs#828.


          fix: allow final word to be wrapped in wrap_help

564c5f0

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 force-pushed the fix-wrapping-bugs branch from 78dec94 to 564c5f0 Compare

January 30, 2017 17:36

coveralls commented Jan 30, 2017 •

edited

Loading

Coverage decreased (-0.008%) to 89.074% when pulling 564c5f0 on mgeisler:fix-wrapping-bugs into 72d7bf3 on kbknapp:master.

coveralls commented Jan 30, 2017

Coverage decreased (-0.008%) to 89.074% when pulling 564c5f0 on mgeisler:fix-wrapping-bugs into 72d7bf3 on kbknapp:master.

3 similar comments

coveralls commented Jan 30, 2017

Coverage decreased (-0.008%) to 89.074% when pulling 564c5f0 on mgeisler:fix-wrapping-bugs into 72d7bf3 on kbknapp:master.

coveralls commented Jan 30, 2017

Coverage decreased (-0.008%) to 89.074% when pulling 564c5f0 on mgeisler:fix-wrapping-bugs into 72d7bf3 on kbknapp:master.

coveralls commented Jan 30, 2017 •

edited

Loading

Coverage decreased (-0.008%) to 89.074% when pulling 564c5f0 on mgeisler:fix-wrapping-bugs into 72d7bf3 on kbknapp:master.

Contributor Author

mgeisler commented Jan 30, 2017

The build failures have been fixed.

mgeisler closed this

mgeisler reopened this

coveralls commented Jan 30, 2017 •

edited

Loading

Coverage decreased (-0.008%) to 89.074% when pulling 564c5f0 on mgeisler:fix-wrapping-bugs into 72d7bf3 on kbknapp:master.

Member

kbknapp commented Jan 30, 2017

Excellent, thanks!

Contributor

homu commented Jan 30, 2017

📌 Commit 564c5f0 has been approved by kbknapp

homu added a commit that referenced this pull request


          Auto merge of #832 - mgeisler:fix-wrapping-bugs, r=kbknapp

2bb4cbe

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 pull request


          Auto merge of #832 - mgeisler:fix-wrapping-bugs, r=kbknapp

e9e01e1

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

Contributor

homu commented Jan 30, 2017

⌛ Testing commit 564c5f0 with merge e9e01e1...

Contributor

homu commented Jan 31, 2017

☀️ Test successful - status

homu merged commit 564c5f0 into clap-rs:master

kbknapp added a commit that referenced this pull request


          fix: fixes a critical bug where subcommand settings were being propog…

8d7115b

…ated incorrectly

Closes #832

kbknapp added a commit that referenced this pull request


          fix: fixes a critical bug where subcommand settings were being propog…

74648c9

…ated too far

Closes #832

kbknapp added a commit that referenced this pull request


          Issues 839,840 (#842)

07d985d

* fix: fixes a critical bug where subcommand settings were being propogated too far

Closes #832

* imp: adds ArgGroup::multiple to the supported YAML fields for building ArgGroups from YAML

Closes #840

* chore: increase version

wildwestrom mentioned this pull request

bump to 1.6.0 Dylan-DPC/clap#1

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet