Skip to content

Conversation

@siame
Copy link

@siame siame commented Nov 4, 2025

Addressing #22546, we want git permalinks to be aware of the current changes within the buffer.

This change calculates how many lines have been added/deleted between the start and end of the selection and uses those values to offset the selection.

This is done within Editor::get_permalink_to_line so that it can be passed to any git_store.

Example:

image

Where this selections permalink would previously return L3-L9, it now returns L2-L7.

Release Notes:

  • git: make permalinks aware of current diffs

Closes #22546


This is my first PR into the zed repository so very happy for any feedback on how I've implemented this. Thanks!

Addressing zed-industries#22546, we want git permalinks to be aware of the current
changes within the buffer.

This change calculates how many lines have been added/deleted between the
start and end of the selection and uses those values to offset the
selection.

This is done within Editor::get_permalink_to_line so that it can be
passed to any git_store.
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Nov 4, 2025
@maxdeviant maxdeviant changed the title git: make permalinks aware of current diffs git: Make permalinks aware of current diffs Nov 4, 2025
@cole-miller
Copy link
Member

Thanks for tackling this! From reading the code, there are a couple of things that need to change:

  • copy permalink to line supports multibuffers, but right now the code to adjust the line range for diff hunks won't work in that situation--it'll iterate over diff hunks in the multibuffer, which may come from buffers unrelated to the one where the selection is.
  • The handling of deleted and modified hunks is not right. For these hunks, the adjustment to make depends on whether the hunks are expanded or not (because expanding and collapsing hunks affects the row_range of MultiBufferDiffHunk, but it shouldn't affect the line range we use for the permalink)

Here is an approach to this feature that I think should avoid these problems:

  • We can still call range_to_buffer_ranges and take the first/last range. This gives us a selection in terms of usize offsets in a single BufferSnapshot, and from here on we shouldn't need to use the original MultiBufferSnapshot at all.
  • Then we need to perform the diff hunk adjustment on that usize buffer range. I think we can do this efficiently (i.e. without iterating over all the diff hunks before the selection) by making some minor changes to the code in the buffer_diff crate and using a sum tree Cursor.

This is probably easier to work through live, would you like to pair on it? Link to book a time (I'll be covering the time slots on this Friday): https://cal.com/esther-trapadoux-zed/30min

@siame
Copy link
Author

siame commented Nov 5, 2025

Thanks for the comments @cole-miller, I've booked in some time to pair on this, looking forward to it.

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

Labels

cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Copy Permalink (GitHub, etc) should better handle modified code

2 participants