Skip to content

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