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

Fix wrapping bugs #832

Merged
merged 4 commits into from
Jan 31, 2017
Merged

Fix wrapping bugs #832

merged 4 commits into from
Jan 31, 2017

Conversation

mgeisler
Copy link
Contributor

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
Copy link

coveralls commented Jan 30, 2017

Coverage Status

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

@kbknapp
Copy link
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).

@mgeisler
Copy link
Contributor Author

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

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.
The &help[j..j] string slice was empty so nothing was shown.
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.
@coveralls
Copy link

coveralls commented Jan 30, 2017

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

3 similar comments
@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

Coverage Status

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

@coveralls
Copy link

coveralls commented Jan 30, 2017

Coverage Status

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

@mgeisler
Copy link
Contributor Author

The build failures have been fixed.

@mgeisler mgeisler closed this Jan 30, 2017
@mgeisler mgeisler reopened this Jan 30, 2017
@coveralls
Copy link

coveralls commented Jan 30, 2017

Coverage Status

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

@kbknapp
Copy link
Member

kbknapp commented Jan 30, 2017

Excellent, thanks!

@homu r+

@homu
Copy link
Contributor

homu commented Jan 30, 2017

📌 Commit 564c5f0 has been approved by kbknapp

homu added a commit that referenced this pull request 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 pull request 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
Copy link
Contributor

homu commented Jan 30, 2017

⌛ Testing commit 564c5f0 with merge e9e01e1...

@homu
Copy link
Contributor

homu commented Jan 31, 2017

☀️ Test successful - status

@homu homu merged commit 564c5f0 into clap-rs:master Jan 31, 2017
kbknapp added a commit that referenced this pull request Feb 3, 2017
kbknapp added a commit that referenced this pull request Feb 3, 2017
kbknapp added a commit that referenced this pull request Feb 3, 2017
* 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 added a commit to wildwestrom/clap that referenced this pull request Jul 29, 2022
Because why the hell not?

1.6.0 (2022-07-05)
New features:
    FEATURE clap-rs#832: Clarify that Captures::len includes all groups, not just matching groups.
    FEATURE clap-rs#857: Add an ExactSizeIterator impl for SubCaptureMatches.
    FEATURE clap-rs#861: Improve RegexSet documentation examples.
    FEATURE clap-rs#877: Upgrade to Unicode 14.
Bug fixes:
    BUG clap-rs#792: Fix error message rendering bug.

1.5.6 (2022-05-20)
    BUG clap-rs#680: Fixes a bug where [[:alnum:][:^ascii:]] dropped [:alnum:] from the class.
    BUG clap-rs#859: Fixes a bug where Hir::is_match_empty returned false for \b.
    BUG clap-rs#862: Fixes a bug where 'ab??' matches 'ab' instead of 'a' in 'ab'.
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 this pull request may close these issues.

4 participants