Implementation advice for collapsing the newly staged section

[jonathanj]

Backstory

In Magit, the staged section follows the collapsed state of the file section (TargetData::Delta in gitu), defaulting to collapsed. Some examples:

• If you stage a whole file, it is collapsed in the staged area
• If you stage a line in a file, then expand the file section in the staged area, then stage the whole file, it is expanded in the staged area (ditto for hunks in a file, etc.)

In gitu, the staged section follows the state of the corresponding unstaged section. So if you expand a file, then decide to stage the whole file, the staged file is remains expanded.

Approach

At a high level I was expecting to implement this along these lines:

• When staging a TargetData::Delta / TargetData::Hunk / TargetData::HunkLine, find the corresponding section for the file/delta in the staged area
• If that staged section is not explicitly expanded (I'm not sure if this state is tracked or can be derived), then make it collapsed

There is some existing code for finding the first hunk item etc, which just searches for the first Item that matches TargetData::Hunk etc. but to refer specifically to a section seems to be something there is not currently any code for, as far as I can tell.

I noticed that Item has an id and implemented collapse_item_by_id, to directly address an item. The staging functions (ops/stage.rs) then attempt to produce the same ID by using the same logic as found when creating the sections in create_diff_items. This appeared to work, because I was just staging entire deltas, but it wouldn't work at all when partially staging files. This is because the id for deltas and hunks is based on the file diff header, which includes an index which uniquely identifies the change. Staging an entire delta "works" because the ID doesn't change.

<render initial UI>
[00:00:03.665] (200010240) INFO   create_diff_items: "diff --git a/src/lib.rs b/src/lib.rs\nindex 0cfbec9..68089ab 100644\n--- a/src/lib.rs\n+++ b/src/lib.rs\n"

<partially stage a hunk, this shows the ID of the file not the hunk, since that's what we want to address>
[00:00:05.670] (200010240) INFO   stage_patch: "diff --git a/src/lib.rs b/src/lib.rs\nindex 0cfbec9..68089ab 100644\n--- a/src/lib.rs\n+++ b/src/lib.rs\n"

<render updated UI>
[00:00:05.768] (200010240) INFO   create_diff_items: "diff --git a/src/lib.rs b/src/lib.rs\nindex bccfcab..68089ab 100644\n--- a/src/lib.rs\n+++ b/src/lib.rs\n"

Problem

So the issue I've run in to is that I can't know how to address the section that will be created by partially staging a file, since I don't know how to generate an identical diff header for the new diff that will be staged after the stage_ action has been executed and the UI updated.

Maybe there is an easy solution to the above that I can't see, but it seems like you would need a way to address any existing staged item for that file in order to compute the new diff anyway.

Possibilities

• Scan the items, keeping track of passing a staged/unstaged item, and performing a partial string match on the ID based on header.new_file
  This seems janky, because it makes assumptions about the order of sections, how the ID is generated, etc.
• Make items more easily addressable
  When staging something, the most direct question I want to ask is "Is this file collapsed in the staged section?", if it were possible to address that section as a composite of each of its parent IDs, then it could be constructed without knowledge of the diff-to-be:

  // I haven't thought too hard about this, just trying to
  // demonstrate what I mean, more than propose an API.
  ItemID::Root("staged_changes")
  ItemID::Delta("staged_changes", "src/lib.rs")
  ItemID::Hunk("staged_changes", "src/lib.rs", hunk_i)
  ItemID::HunkLine("staged_changes", "src/lib.rs", hunk_i, line_i)
  
  // Swap pieces out, etc.
  ItemID::Delta("staged_changes", "src/lib.rs").replace_root(ItemID::Root("unstaged_changes"))
  

Blind spots

• I'm not a Rust programmer, I may suggest things that make no sense in Rust, apologies in advance for that
• I'm coming in to this not having given too much thought to how Magit works internally, I may just be completely wrong and this is a 2 minute solution. Which would be great to discover
• I haven't really thought about how this affects branches/commits/stashes, but I think that new ItemID cases could be added for those