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

Adds range syntax for line highlights #809

Merged
merged 2 commits into from
Jan 27, 2020
Merged

Adds range syntax for line highlights #809

merged 2 commits into from
Jan 27, 2020

Conversation

lkalir
Copy link
Contributor

@lkalir lkalir commented Jan 23, 2020

--highlight-line now accepts line ranges like the --line-range option.

Copy link
Owner

@sharkdp sharkdp left a comment

Choose a reason for hiding this comment

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

Great, thank you very much!

I have a few minor comments.

'--highlight-line 30:40' highlights lines 30 to 40\n \
'--highlight-line :40' highlights lines 1 to 40\n \
'--highlight-line 40:' highlights lines 40 to the end of the file\n \
'--highlight-line 40' highlights only line 40",
Copy link
Owner

Choose a reason for hiding this comment

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

Can we please move this example to the top? Also, highlights only line 40 => highlight line 40

@@ -328,7 +333,8 @@ pub fn build_app(interactive_output: bool) -> ClapApp<'static, 'static> {
For example:\n \
'--line-range 30:40' prints lines 30 to 40\n \
'--line-range :40' prints lines 1 to 40\n \
'--line-range 40:' prints lines 40 to the end of the file",
'--line-range 40:' prints lines 40 to the end of the file\n \
'--line-range 40' prints only line 40",
Copy link
Owner

Choose a reason for hiding this comment

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

minor: "only prints line 40"

new_range.upper = line_numbers[1].parse()?;
Ok(new_range)
},
_ => Err("expected at most single ':' character".into()),
Copy link
Owner

Choose a reason for hiding this comment

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

Can we change this error message to:

Line range contained more than one ':' character. Expected format: 'N' or 'N:M'.

@lkalir lkalir requested a review from sharkdp January 27, 2020 00:47
@sharkdp
Copy link
Owner

sharkdp commented Jan 27, 2020

Thank you

@sharkdp sharkdp merged commit 5ef1c6c into sharkdp:master Jan 27, 2020
@sharkdp
Copy link
Owner

sharkdp commented Mar 22, 2020

This has been released in bat 0.13.

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.

2 participants