Skip to content

fix SBS navigation - #384

Open
natebrennand wants to merge 2 commits into
agavra:mainfrom
natebrennand:nate/fix-sbs-navigation
Open

fix SBS navigation#384
natebrennand wants to merge 2 commits into
agavra:mainfrom
natebrennand:nate/fix-sbs-navigation

Conversation

@natebrennand

Copy link
Copy Markdown
Contributor

The zz, zb and expand diffs features were broken on main when using side-by-side diffs. This fixes all the issues I identified and adds some tests that should help mitigate future regressions 🤞🏻.

@agavra agavra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM, one thing that's probably worth making consistnetn across SBS and unified instead of accounting for it

Comment thread src/app.rs
// blank line above, so no extra Spacing is needed here.
// File header: unified mode uses a 3-line box (border+name+border),
// side-by-side uses a single ═══ header line.
// The annotation count must match the renderer's visual line count.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ugh I totally forgot to test this change on the side-by-side mode. I think a better fix here is probably to render the headers the same way (e.g. a box for all of it).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

alternatively I'm happy going tback to just a single header line for unified as well. I don't know if I actually love the new boxes

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