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.

Sign up for GitHub

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

Jump to bottom

Issue 428 redux #449

Merged
merged 7 commits into from
Mar 15, 2016
Merged

Issue 428 redux #449

merged 7 commits into from
Mar 15, 2016

Conversation

kbknapp
Copy link
Member

@kbknapp kbknapp commented Mar 14, 2016

No description provided.

@yo-bot
Copy link

yo-bot commented Mar 14, 2016

r? @Vinatorul

(yo-bot has picked a reviewer for you, use r? to override)

@kbknapp
feat(Help Message): can auto wrap and aligning help text to term width
e36af02 
By default `clap` now automatically wraps and aligns help strings to the
term width. i.e.

```
    -o, --option <opt>    some really long help
text that should be auto aligned but isn't righ
t now
```

Now looks like this:

```
    -o, --option <opt>    some really long help
                          text that should be
                          auto aligned but isn't
                          right now
```

The wrapping also respects words, and wraps at spaces so as to not cut
words in the middle.

This requires the `libc` dep which is enabled (by default) with the
`wrap_help` cargo feature flag.

Closes #428
@kbknapp
tests: moves some \t tabs to four spaces for consistency
e428bb6
@kbknapp
tests: moves app settings tests to the proper place
675c39f
@kbknapp
chore: increase version
387b147
sru
sru reviewed Mar 14, 2016
View reviewed changes
src/args/arg_builder/flag.rs
write_arg_help!(@flag self, w, tab, longest, nlh);
write!(w, "\n")
pub fn write_help<W: io::Write>(&self, w: &mut W, longest: usize, nlh: bool) -> io::Result<()> {
let hw = HelpWriter::new(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there reason for 1 line per argument?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, missed that. Thanks

@kbknapp
Copy link
Member Author

kbknapp commented Mar 14, 2016

@sru or @Vinatorul I'll let you do the honors of the merge once you give it a quick once over.

@kbknapp
Copy link
Member Author

kbknapp commented Mar 15, 2016

Actually, don't merge yet, I'm gonna see if I can fix one thing first.

Edit: nevermind, didn't pan out. Safe to merge 👍

@sru
Copy link
Contributor

sru commented Mar 15, 2016

Can you make one term.rs file each for windows and not windows? I think you could do it by:

mod term_windows;
use term_windows as term;

Something like that

EDIT: of course, with appropriate #[] things. I think it would make each file clearer.

@kbknapp
Copy link
Member Author

kbknapp commented Mar 15, 2016

@sru I tried this at first - but it caused a few issues with cfg attributes. So I'd rather just merge it in the current state, and start working on the windows implementation at a later point...perhaps with per platform sub modules (inside term) but re-exported to term::func_name although that's something I'll still be thinking about the design for a bit and shouldn't affect he public API so it shouldn't be too big an issue today.

@kbknapp kbknapp mentioned this pull request Mar 15, 2016
Merged
@kbknapp
Copy link
Member Author

kbknapp commented Mar 15, 2016

I'm gonna merge cause I've got some vacation coming up. This way I can make sure all is good and no maJoe bugs before I leave for a few days 😉

@kbknapp
Copy link
Member Author

kbknapp commented Mar 15, 2016

@homu r+

@homu
Copy link
Contributor

homu commented Mar 15, 2016

📌 Commit 031b717 has been approved by kbknapp

homu added a commit that referenced this pull request Mar 15, 2016
@homu
Auto merge of #449 - kbknapp:issue-428-redux, r=kbknapp
f6dbc39 
Issue 428 redux
homu added a commit that referenced this pull request Mar 15, 2016
@homu
Auto merge of #449 - kbknapp:issue-428-redux, r=kbknapp
f6d1685 
Issue 428 redux
@homu
Copy link
Contributor

homu commented Mar 15, 2016

⌛ Testing commit 031b717 with merge f6d1685...

@homu
Copy link
Contributor

homu commented Mar 15, 2016

☀️ Test successful - status

@homu homu merged commit 031b717 into master Mar 15, 2016
homu added a commit that referenced this pull request Mar 15, 2016
@homu
Auto merge of #450 - kbknapp:issue-416, r=kbknapp
36e8d78 
Issue 416 - Help subcommand accepts other subcommands as arguments

Merge *after* #449

Closes #416
homu added a commit that referenced this pull request Mar 15, 2016
@homu
Auto merge of #450 - kbknapp:issue-416, r=kbknapp
cc127f9 
Issue 416 - Help subcommand accepts other subcommands as arguments

Merge *after* #449

Closes #416
@kbknapp kbknapp deleted the issue-428-redux branch March 21, 2016 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Vinatorul Vinatorul

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kbknapp @yo-bot @sru @homu @Vinatorul