zed icon indicating copy to clipboard operation
zed copied to clipboard

Extra horizontal scrolling when inline blame is enabled with soft wrapping

Open AaronFeickert opened this issue 10 months ago • 8 comments

Summary

If inline blame and soft wrap are both enabled, unnecessarily extra horizontal scroll padding is added if any line soft wraps (or would otherwise wrap if inline blame were to appear).

Steps to trigger the problem:

  1. Enable both inline blame and soft wrapping.
  2. Open a project with a corresponding git repository.
  3. Open a file in the project with a line that soft wraps.

Actual Behavior: Even when the cursor is not on a wrapping line, it is possible to scroll horizontally. If the cursor is on a wrapping line, inline blame appears to fill this extra scroll padding.

Expected Behavior: Horizontal scrolling should not occur.

Zed Version and System Specs

Zed: v0.173.6 (Zed Preview) OS: macOS 15.3.1 Memory: 16 GiB Architecture: aarch64

AaronFeickert avatar Feb 12 '25 20:02 AaronFeickert

Thanks for the report @AaronFeickert - I can reproduce this.

JosephTLyons avatar Feb 12 '25 20:02 JosephTLyons

On further inspection, it's not quite that the line in question must wrap. If the combination of a line and an associated inline blame would wrap, the extra padding is added. Presumably this occurs because the blame is never wrapped; it always appears on the first of any wrapping portions of a line. So a wrapping line will trigger this, but it's not strictly necessary.

AaronFeickert avatar Feb 12 '25 21:02 AaronFeickert

@JosephTLyons: Any thoughts on what the correct behavior should be for this? I can think of a couple approaches, but I'm not sure which (if either) is preferable:

  • The horizontal scrollbar should only appear (and allow scrolling) after clicking a line to make its inline blame appear. This has the downside of violating the unwritten "no scrolling with soft wrap enabled" rule.
  • Inline blame should itself soft wrap if needed, similarly to inlay hints. This might be more confusing to the user, since the blame location feels less consistent than with the current approach.

AaronFeickert avatar Feb 14 '25 20:02 AaronFeickert

The horizontal scrollbar should only appear (and allow scrolling) after clicking a line to make its inline blame appear. This has the downside of violating the unwritten "no scrolling with soft wrap enabled" rule.

fwiw I think this would be preferable.

bcdanieldickison avatar Feb 14 '25 21:02 bcdanieldickison

In my opinion you could try this: if there if not enough space to fit the blame, you place it in a different line, such as the one below, with an arrow pointing up, potentially overlapping with the text in the line below. Maybe with a frame (implemented like a hover?) in this case. OR in the same line but away from the cursor.

mocenigo avatar Feb 19 '25 11:02 mocenigo

Thanks, @AaronFeickert, for raising this issue. it's been on my radar for a while. I think the right approach is to display it as inlay hints, adhering to the "no scrolling with soft wrap" rule.

The first approach you mentioned has a bunch of drawbacks, like the scrollbar width constantly changing when navigating up/down. Again, scrolling should simply not happen with soft wrap.

I'll pick this up soon, as it's something I also want to fix eagerly, but I can't specify a timeline.

smitbarmase avatar Feb 20 '25 06:02 smitbarmase

@0xtimsb: sounds good! I also think that the inlay approach seems like the cleaner design.

AaronFeickert avatar Feb 20 '25 14:02 AaronFeickert

@smitbarmase: Is this still something you'd like to tackle yourself?

AaronFeickert avatar Apr 30 '25 21:04 AaronFeickert

On further inspection, it's not quite that the line in question must wrap. If the combination of a line and an associated inline blame would wrap, the extra padding is added.

For me the scrollbar appears/change even when there's plenty of space for both: (soft wrap on). The file buffer contain other soft-wrapped lines though.

Image

Image

Simple workaround for now: just hide the horizontal scollbar:

"scrollbar": {
  "axes": {
      "horizontal": false,
  }
}

olejorgenb avatar Jun 25 '25 22:06 olejorgenb

I'm happy to take this on if no one else is working on it

esimkowitz avatar Jun 26 '25 03:06 esimkowitz

I think that this issue is more philosophical than I originally thought. The "bug" here was introduced by #23374 to solve an issue where inline blames weren't scrollable when wrapping was enabled. The same issue is present still for inline diagnostics and makes me think that we should approach this instead by refactoring how we insert inline elements, at least those that come to the right of content.

I think we should move the rendering of these inline elements into the DisplayMap layout code and factor them in when calculating wrapping. The data needed for both the diagnostics and the blame are present in the mapped RowInfo already so the only context that needs to be moved is where we track the active rows. I think this makes more sense in DisplayMap anyway so I feel like it's worth it. The downside of this change is that we currently only render inline elements when they're visible, but now I think we'd at least to lay them out in order to determine the scroll height and to avoid inconsistencies with the Minimap.

I'm curious for input, especially from @SomeoneToIgnore and @MrSubidubi

esimkowitz avatar Jul 06 '25 02:07 esimkowitz

I would stay away from touching DisplayMap as long as possible: the code is mission-critical and relatively complex. While some parts of the BlockMap and InlayMap do a bit of extra, almost all of the related code deals with particular file's text and transformations on top of that particular text.

In that paradigm, the diagnostics coming from LSP are quite alien with their own alien lifecycle — cramping those new concepts into the DisplayMap is possible, but won't be easy. And what for? (as another "philosophical question").


New active diagnostics (appear on f8/shift-f8) are perfectly capable of rendering themselves at the edge without oddities, at least for "simple" cases:

Image

and mass "simple" cases such as project diagnostics view (cmd-shift-m):

Image

This, but with less styling seems to be exactly what we want for both inline blame and inline diagnostics?

We sure do need to think of how inline and active diagnostics live along in this new world, but that seems much simpler to approach compared to wrecking buffer text in DisplayMap?

SomeoneToIgnore avatar Jul 06 '25 09:07 SomeoneToIgnore

I've noticed this recently, no horizontal scroll bar without inline blame turned on, with it turned on, it's causing the scroll bar to appear whenever the blame text is there (even if the changes are unstaged, and no blame message)

It only seems to happen when there's a pretty long line in the file that's almost hitting the edge of the screen, in other files I don't see it happening

Also flickers during typing, not just mouse clicks

https://github.com/user-attachments/assets/0d4989b9-9a5c-414a-b122-ab1a0bf63afe

mxz7 avatar Nov 29 '25 19:11 mxz7