-
Notifications
You must be signed in to change notification settings - Fork 6k
Fix track file renames in git panel #42352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix track file renames in git panel #42352
Conversation
|
Thanks! This is a good start, but I think we should find a way to show both the old and the new paths in the panel. That probably requires a change to how we track statuses in the data layer, so we can store an additional path for things that were renamed. One strategy that I think might make sense would be to add an additional map between old paths and new paths to |
@cole-miller what do you think about this? |
|
@ddoemonn That looks reasonable to me, though I do want to have our design team weigh in before we merge this (and in the meantime we can get the data layer ready to go). |
|
@cole-miller What do you think about this approach except design for now? |
|
Thanks! The backend side of this looks pretty good to me now. It would be good to test (if you haven't already) in what cases git emits the renamed status code for the index/working copy/both, and check that we show the right thing in all cases. Also that staging/unstaging/restoring/committing/uncommitting with renamed paths all have reasonable behavior. As for the design, tagging @mattermill for his thoughts, but one big question for me is how this interacts with #41857. |
|
@cole-miller is there anything you want me to update about this PR? |
|
@ddoemonn This looks reasonable to me! However, as @cole-miller noted, we will likely have to revisit once we move on to #41857. |
This reverts commit b0a7def.
|
@ddoemonn Sorry for merging this prematurely, it seems like there are still some bugs:
If you open a new PR we can advise on how to go about fixing these! |
|
Hey @cole-miller! Thanks for clarifying these issues. I actually experienced these bugs after the PR was already closed, so I didn’t realize they were happening beforehand. I’m a little bit confused about these parts too, to be honest. I can definitely open a new PR to fix them, but I’m a bit busy at work today. How urgent is this? I want to make sure I prioritize it appropriately. |
|
@cole-miller new PR is here #43077 |

Closes #30549
Release Notes: