Skip to content

Commit

Permalink
imp(Help Wrapping): clap now ignores hard newlines in help messages a…
Browse files Browse the repository at this point in the history
…nd properly re-aligns text, but still wraps if the term width is too small

Prior to this commit, clap would mangle help messages with hard newlines
(see #617). After this commit, clap will ignore hard newlines and treat
them like an inserted newline, properly wrapping and aligning text and
then restarting it's count until the next newline should be inserted.

This commit also removes the need for using `{n}` to insert a newline in
help text, now the traditional `\n` can be used. For backwards
compatibility, the `{n}` still works.

Closes #617
  • Loading branch information
kbknapp committed Sep 7, 2016
1 parent 0d50350 commit c767852
Showing 1 changed file with 35 additions and 15 deletions.
50 changes: 35 additions & 15 deletions src/app/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,11 @@ impl<'a> Help<'a> {
}
lw
};
help = if help.contains("{n}") {
help.replace("{n}", "\n")
} else {
help
};

This comment has been minimized.

Copy link
@vks

vks Sep 7, 2016

Contributor

Why not just use replace unconditionally? It seems to me that a noop replace is more or less computationally as much work as a negative contains check. (This concerns a few other lines in this commit as well.)

This comment has been minimized.

Copy link
@nabijaczleweli

nabijaczleweli Sep 7, 2016

Contributor

Yeah, unconditional replace has more or equal performance in the general case

This comment has been minimized.

Copy link
@kbknapp

kbknapp Sep 7, 2016

Author Member

@vks @nabijaczleweli I agree...I'm not sure why I wrote it like that other than lack of thinking much about it. We can change that on the next PR, or someone would like to throw a quick commit together I'm good with it.

This comment has been minimized.

Copy link
@nabijaczleweli

nabijaczleweli Sep 7, 2016

Contributor
wrap_help(&mut help, longest_w, width);
} else {
sdebugln!("No");
Expand Down Expand Up @@ -445,6 +450,11 @@ impl<'a> Help<'a> {
}
lw
};
help = if help.contains("{n}") {
help.replace("{n}", "\n")
} else {
help
};
wrap_help(&mut help, longest_w, avail_chars);
} else {
sdebugln!("No");
Expand All @@ -458,11 +468,11 @@ impl<'a> Help<'a> {
help.push_str(&*spec_vals);
&*help
};
if help.contains("{n}") {
if let Some(part) = help.split("{n}").next() {
if help.contains("\n") {
if let Some(part) = help.split("\n").next() {
try!(write!(self.writer, "{}", part));
}
for part in help.split("{n}").skip(1) {
for part in help.split("\n").skip(1) {
try!(write!(self.writer, "\n"));
if nlh || force_next_line {
try!(write!(self.writer, "{}{}{}", TAB, TAB, TAB));
Expand Down Expand Up @@ -920,27 +930,37 @@ fn wrap_help(help: &mut String, longest_w: usize, avail_chars: usize) {
sdebugln!("Yes");
let mut prev_space = 0;
let mut j = 0;
let mut i = 0;
// let mut i = 0;

This comment has been minimized.

Copy link
@vks

vks Sep 7, 2016

Contributor

Can this be removed? There is also some more code that is commented out.

This comment has been minimized.

Copy link
@kbknapp

kbknapp Sep 7, 2016

Author Member

It could be, typically once I comment out code I'll leave it for a single patch release cycle in case bugs pop up from the change then I remove it. I know I can go back in git history, but this is a quick way to see what I was working on.

This comment has been minimized.

Copy link
@vks

vks Sep 7, 2016

Contributor

Well, I don't really see how that is better than using git log/git blame, but if it works for you, why not.

for (idx, g) in (&*help.clone()).grapheme_indices(true) {
debugln!("iter;idx={},g={}", idx, g);
if g != " " {
continue;
}
if str_width(&help[j..idx + (2 * i)]) < avail_chars {
debugln!("Still enough space...");
if g == "\n" {
debugln!("Newline found...");
debugln!("Still space...{:?}", str_width(&help[j..idx]) < avail_chars);
if str_width(&help[j..idx]) < avail_chars {
// i += 1;
j = idx; // + i;
continue;
}
} else if g != " " {
if idx != help.len() - 1 || str_width(&help[j..idx]) < avail_chars {
continue;
}
debugln!("Reached the end of the line and we're over...");
} else if str_width(&help[j..idx]) < avail_chars { //(2 * i)]) < avail_chars {
debugln!("Space found with room...");
prev_space = idx;
continue;
}
debugln!("Adding Newline...");
j = prev_space + (2 * i);
debugln!("i={},prev_space={},j={}", i, prev_space, j);
j = prev_space; // + i;//(2 * i);
debugln!("prev_space={},j={}", prev_space, j);
debugln!("removing: {}", j);
debugln!("char at {}: {}", j, &help[j..j]);
help.remove(j);
help.insert(j, '{');
help.insert(j + 1, 'n');
help.insert(j + 2, '}');
i += 1;
help.insert(j, '\n');
// help.insert(j + 1, 'n');
// help.insert(j + 2, '}');
// i += 1;
}
} else {
sdebugln!("No");
Expand Down

0 comments on commit c767852

Please sign in to comment.