Skip to content

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

'--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