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

use saturating substraction to calculate Line ranges #1891

Merged
merged 3 commits into from Oct 17, 2021

Conversation

divagant-martian
Copy link
Contributor

Fixes #1869

Running the bat on debug mode with the repository that reproduces the issue shows it's an underflow when getting the first line to show, accounting for the surrounding number of lines.

Harder to notice because it only affects the first line, but this makes it so that the first lines are not shown unless they belong to the range of another changed line.

@divagant-martian
Copy link
Contributor Author

This solves the visible issue, but it's still possible to create a LineRange that (in release) has a starting point ahead of the end point. Let me know you think it's best to solve it that way. Also maybe a test would be nice but tbh I'm not sure how are those managed in bat

@keith-hall
Copy link
Collaborator

Thanks for your contribution!

Also maybe a test would be nice but tbh I'm not sure how are those managed in bat

I'm no expert (I'm a maintainer, but I mainly deal with the syntax highlighting side of bat), but I think you could modify the first line of this file:

struct Rectangle {

(i.e. rename Rectangle to Rect or something, or just add a comment)

and then the snapshot tests which do a git diff would pick up a change on the first line. Then I guess it'd be a case of updating those output files, so that CI would pass.

@divagant-martian
Copy link
Contributor Author

divagant-martian commented Oct 15, 2021

I used generate_snapshots.py and it did the trick. I tried instead of adding a doc comment (new first line), simply changing the line (adding pub) but the number of changes is the same. Some files appear completely changed, not sure why. If CI passes with those I guess it's fine?

EDIT: never mind with the large amount of changes, those are correct.

@sharkdp sharkdp merged commit 2339d78 into sharkdp:master Oct 17, 2021
@sharkdp
Copy link
Owner

sharkdp commented Oct 17, 2021

Thank you very much!

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.

diff mode doesn't display content
3 participants