Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Changes from renamed files are ignored #6014

Open
saschanaz opened this issue Oct 27, 2018 · 10 comments · May be fixed by #17467
Open

Changes from renamed files are ignored #6014

saschanaz opened this issue Oct 27, 2018 · 10 comments · May be fixed by #17467
Labels
investigation-needed Likely bugs, but haven't been reliably reproduced by a reviewer

Comments

@saschanaz
Copy link
Contributor

saschanaz commented Oct 27, 2018 •
edited

Description

Files can simultaneously be renamed and changed, but GitHub Desktop ignores content changes from renamed files.

Version

  • GitHub Desktop: 1.4.3
  • Operating system: Windows 10

Steps to Reproduce

  1. Commit a file with 100 lines.
  2. Rename the file.
  3. Change a single line on the file.
  4. git add the file and see what GitHub Desktop says.

Expected Behavior

GitHub Desktop should show content changes, as VSCode does:

image

Actual Behavior

It ignores any content change.

image

Additional Information

Logs
@shiftkey shiftkey added the investigation-needed Likely bugs, but haven't been reliably reproduced by a reviewer label Oct 29, 2018
@tierninho
Copy link
Contributor

tierninho commented May 8, 2019 •
edited

Can reproduce this issue on 1.7.0-beta6, mac.

Not sure if this error was related but it popped up at the same time:

http.ts:135 HEAD https://api.github.com/repos/tierninho/-/git 0 ()
t.request @ http.ts:135
request @ api.ts:559
getFetchPollInterval @ api.ts:572
getFetchInterval @ background-fetcher.ts:130
performAndScheduleFetch @ background-fetcher.ts:112
stopped.timeoutHandle.window.setTimeout @ background-fetcher.ts:120
setTimeout (async)
performAndScheduleFetch @ background-fetcher.ts:116
async function (async)
performAndScheduleFetch @ background-fetcher.ts:88
start @ background-fetcher.ts:70
startBackgroundFetching @ app-store.ts:1526
_selectRepositoryRefreshTasks @ app-store.ts:1336
_selectRepository @ app-store.ts:1246
async function (async)
_selectRepository @ app-store.ts:1203
selectRepository @ dispatcher.ts:219
Re.onSelectionChanged @ app.tsx:2310
onItemClick @ repositories-list.tsx:178
onRowClick @ filter-list.tsx:365
onRowClick @ list.tsx:972
onRowClick @ list-row.tsx:62
r @ react-dom.production.min.js:15
invokeGuardedCallback @ react-dom.production.min.js:16
invokeGuardedCallbackAndCatchFirstError @ react-dom.production.min.js:16
p @ react-dom.production.min.js:20
h @ react-dom.production.min.js:22
g @ react-dom.production.min.js:22
c @ react-dom.production.min.js:21
S @ react-dom.production.min.js:24
v @ react-dom.production.min.js:24
Kt @ react-dom.production.min.js:83
batchedUpdates @ react-dom.production.min.js:187
nt @ react-dom.production.min.js:42
Qt @ react-dom.production.min.js:85
interactiveUpdates @ react-dom.production.min.js:188
Xt @ react-dom.production.min.js:84
install.ts:27 getFetchPollInterval: failed for tierninho/-
TypeError: Failed to fetch

This is a UI issue, as the changes to the file are surfaced in the History tab and not lost.

@Nyameliaaaa
Copy link

This Issue Occurs On All Platforms.

@ad-si
Copy link

ad-si commented Apr 6, 2021

This is an absolute must have feature. Lack thereof makes GitHub Desktop unusable. I just missed some changes because the file was also renamed at the same time and GitHub Desktop just says The file was renamed but not changed.

@saschanaz
Copy link
Contributor Author

saschanaz commented Apr 6, 2021 •
edited

desktop/app/src/ui/diff/index.tsx

Lines 210 to 216 in 0fec3ad

if (this.props.file.status.kind === AppFileStatusKind.Renamed) {
return (
<div className="panel renamed">
The file was renamed but not changed
</div>
)
}

This lines seemingly think that renamed files are always unchanged.

Edit: Ah no, diff.hunks.length === 0 means something upper than that is broken.

@sergiou87 sergiou87 mentioned this issue Jun 24, 2021
Closed
@sergiou87
Copy link
Member

Another instance of this issue in #12501

ssigwart added a commit to ssigwart/github-desktop that referenced this issue Sep 9, 2023
@ssigwart
Fix staged file renamed with changes showing no diff
3c03ab8 
- Closes desktop#6014
- If a file is renamed with changes and is staged, both the rename and differences will be shown with this update.
- If a file is renamed is staged, but not all changes, a warning message is shown that the file must be staged. Otherwise, I would have to choose between showing the diff for the staged or unstaged changes, but not both.
@ssigwart ssigwart mentioned this issue Sep 9, 2023
Closed
@ssigwart
Copy link

ssigwart commented Sep 9, 2023

I just submitted #17364 which solves this. However, if there are both staged and unstaged changes to a renamed file, I couldn't figure out how to show both of them, but I added a message saying that you need to stage all the changes before you can see the diff. That seems much better than the current state that says that there were no changes.

@ssigwart
Copy link

@sergiou87, I see that app/src/lib/status-parser.ts sets workingTree to GitStatusEntry.Modified :

desktop/app/src/lib/status-parser.ts

Lines 322 to 329 in a1ece18

if (statusCode === 'RM') {
return {
kind: 'renamed',
index: GitStatusEntry.Renamed,
workingTree: GitStatusEntry.Modified,
submoduleStatus,
}
}

If I could find a way to pass through that field to the diff renderer, I could have it change the message to " The file was renamed and modified."

desktop/app/src/ui/diff/index.tsx

Lines 210 to 216 in 0fec3ad

if (this.props.file.status.kind === AppFileStatusKind.Renamed) {
return (
<div className="panel renamed">
The file was renamed but not changed
</div>
)
}

Is that a good approach?

@sergiou87
Copy link
Member

I think so, as long as this change is introduced in a safe way that doesn't affect the rest of well-supported scenarios. It definitely feels more reliable than the previous one 😄

ssigwart added a commit to ssigwart/github-desktop that referenced this issue Sep 30, 2023
@ssigwart
Fix staged file renamed with changes showing no diff
83db3b6 
- Closes desktop#6014
- Fixes the case where a staged file is moved and modified. Previously, it would say it was moved with no changes.
@ssigwart ssigwart linked a pull request Sep 30, 2023 that will close this issue
Open
@ssigwart
Copy link

Thanks, @sergiou87. I opened #17467. It's still a bit of passing around fields, but doesn't mess with the actual git diff command, so hopefully this PR is better.

@0xdevalias
Copy link

0xdevalias commented Dec 17, 2023 •
edited

See also:

@0xdevalias 0xdevalias mentioned this issue Dec 17, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
investigation-needed Likely bugs, but haven't been reliably reproduced by a reviewer
Projects
None yet
Milestone
No milestone
8 participants
@shiftkey @0xdevalias @sergiou87 @ssigwart @saschanaz @tierninho @Nyameliaaaa @ad-si