Skip to content
This repository was archived by the owner on Sep 8, 2018. It is now read-only.

Conversation

nathansobo
Copy link
Contributor

Description of the Change

This PR adds special handling for the first line of a file being deleted. Currently, the git-line-removed class always adds decoration to the bottom of the line number. This works well for everything except when the first line is deleted. In that case, we attempt to decorate row -1, which gets clipped to 0. Then the decorations make it look like we've deleted the second line instead of the first. Now if we need to decorate row -1, we instead add git-previous-line-removed class, which duplicates the existing style except for the positioning of the decoration.

Benefits

Correctly indicates when the first line has been deleted.

Alternate Designs

We could have changed the git-line-removed styling to always apply to always render the decoration at the top and apply the decoration to the next line instead. That was a bigger change and seems like it could have issues at the end of the file instead.

Possible Drawbacks

The duplicated style could maybe be done more elegantly with LESS. @simurai I'll defer to you on that on.

Applicable Issues

Closes #128

@nathansobo nathansobo requested a review from simurai January 16, 2018 19:24
@simurai
Copy link
Contributor

simurai commented Jan 17, 2018

The duplicated style could maybe be done more elegantly with LESS. @simurai I'll defer to you on that on.

  • I combined the two classes so they can share most of the properties. Not really needed, maybe just easier to read.
  • Also styled it when enabling icons.
    screen shot 2018-01-17 at 5 18 24 pm

Otherwise this looks good. 👍

@nathansobo nathansobo merged commit 2621b24 into master Jan 18, 2018
@nathansobo nathansobo deleted the ns-fix-first-line branch January 18, 2018 16:36
@nathansobo
Copy link
Contributor Author

Thanks for your help @simurai.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants