Merge pull request desktop#17467 from ssigwart/renameChanged2

Fix staged file renamed with changes showing no diff

app/src/lib/git/log.ts

@@ -69,20 +69,40 @@ function mapStatus(
     return { kind: AppFileStatusKind.Deleted, submoduleStatus }
   } // deleted
   if (status === 'R' && oldPath != null) {
-    return { kind: AppFileStatusKind.Renamed, oldPath, submoduleStatus }
+    return {
+      kind: AppFileStatusKind.Renamed,
+      oldPath,
+      submoduleStatus,
+      renameIncludesModifications: false,
+    }
   } // renamed
   if (status === 'C' && oldPath != null) {
-    return { kind: AppFileStatusKind.Copied, oldPath, submoduleStatus }
+    return {
+      kind: AppFileStatusKind.Copied,
+      oldPath,
+      submoduleStatus,
+      renameIncludesModifications: false,
+    }
   } // copied
 
   // git log -M --name-status will return a RXXX - where XXX is a percentage
   if (status.match(/R[0-9]+/) && oldPath != null) {
-    return { kind: AppFileStatusKind.Renamed, oldPath, submoduleStatus }
+    return {
+      kind: AppFileStatusKind.Renamed,
+      oldPath,
+      submoduleStatus,
+      renameIncludesModifications: status !== 'R100',
+    }
   }
 
   // git log -C --name-status will return a CXXX - where XXX is a percentage
   if (status.match(/C[0-9]+/) && oldPath != null) {
-    return { kind: AppFileStatusKind.Copied, oldPath, submoduleStatus }
+    return {
+      kind: AppFileStatusKind.Copied,
+      oldPath,
+      submoduleStatus,
+      renameIncludesModifications: false,
+    }
   }
 
   return { kind: AppFileStatusKind.Modified, submoduleStatus }

app/src/lib/git/status.ts

@@ -155,12 +155,17 @@ function convertToAppStatus(
       kind: AppFileStatusKind.Copied,
       oldPath,
       submoduleStatus: entry.submoduleStatus,
+      renameIncludesModifications: false,
     }
   } else if (entry.kind === 'renamed' && oldPath != null) {
     return {
       kind: AppFileStatusKind.Renamed,
       oldPath,
       submoduleStatus: entry.submoduleStatus,
+      renameIncludesModifications:
+        entry.workingTree === GitStatusEntry.Modified ||
+        (entry.renameOrCopyScore !== undefined &&
+          entry.renameOrCopyScore < 100),
     }
   } else if (entry.kind === 'untracked') {
     return {
@@ -276,7 +281,11 @@ function buildStatusMap(
   entry: IStatusEntry,
   conflictDetails: ConflictFilesDetails
 ): Map<string, WorkingDirectoryFileChange> {
-  const status = mapStatus(entry.statusCode, entry.submoduleStatusCode)
+  const status = mapStatus(
+    entry.statusCode,
+    entry.submoduleStatusCode,
+    entry.renameOrCopyScore
+  )
 
   if (status.kind === 'ordinary') {
     // when a file is added in the index but then removed in the working

app/src/lib/status-parser.ts

@@ -28,6 +28,9 @@ export interface IStatusEntry {
 
   /** The original path in the case of a renamed file */
   readonly oldPath?: string
+
+  /** The rename or copy score in the case of a renamed file */
+  readonly renameOrCopyScore?: number
 }
 
 export function isStatusHeader(
@@ -141,6 +144,7 @@ function parsedRenamedOrCopiedEntry(
     statusCode: match[1],
     submoduleStatusCode: match[2],
     oldPath,
+    renameOrCopyScore: parseInt(match[8].substring(1), 10),
     path: match[9],
   }
 }
@@ -196,7 +200,8 @@ function mapSubmoduleStatus(
  */
 export function mapStatus(
   statusCode: string,
-  submoduleStatusCode: string
+  submoduleStatusCode: string,
+  renameOrCopyScore: number | undefined
 ): FileEntry {
   const submoduleStatus = mapSubmoduleStatus(submoduleStatusCode)
 
@@ -269,6 +274,7 @@ export function mapStatus(
       kind: 'renamed',
       index: GitStatusEntry.Renamed,
       workingTree: GitStatusEntry.Unchanged,
+      renameOrCopyScore,
       submoduleStatus,
     }
   }
@@ -278,6 +284,7 @@ export function mapStatus(
       kind: 'renamed',
       index: GitStatusEntry.Unchanged,
       workingTree: GitStatusEntry.Renamed,
+      renameOrCopyScore,
       submoduleStatus,
     }
   }
@@ -325,6 +332,7 @@ export function mapStatus(
       kind: 'renamed',
       index: GitStatusEntry.Renamed,
       workingTree: GitStatusEntry.Modified,
+      renameOrCopyScore,
       submoduleStatus,
     }
   }
@@ -334,6 +342,7 @@ export function mapStatus(
       kind: 'renamed',
       index: GitStatusEntry.Renamed,
       workingTree: GitStatusEntry.Deleted,
+      renameOrCopyScore,
       submoduleStatus,
     }
   }

app/src/models/status.ts

@@ -47,6 +47,7 @@ export type PlainFileStatus = {
 export type CopiedOrRenamedFileStatus = {
   kind: AppFileStatusKind.Copied | AppFileStatusKind.Renamed
   oldPath: string
+  renameIncludesModifications: boolean
   submoduleStatus?: SubmoduleStatus
 }
 
@@ -146,6 +147,8 @@ type RenamedOrCopiedEntry = {
   readonly workingTree?: GitStatusEntry
   /** the submodule status for this entry */
   readonly submoduleStatus?: SubmoduleStatus
+  /** The rename or copy score in the case of a renamed file */
+  readonly renameOrCopyScore?: number
 }
 
 export enum UnmergedEntrySummary {

app/src/ui/diff/index.tsx

@@ -31,6 +31,8 @@ import { BinaryFile } from './binary-file'
 import { SideBySideDiff } from './side-by-side-diff'
 import { IFileContents } from './syntax-highlighting'
 import { SubmoduleDiff } from './submodule-diff'
+import { Octicon } from '../octicons'
+import * as OcticonSymbol from '../octicons/octicons.generated'
 
 // image used when no diff is displayed
 const NoDiffImage = encodePathAsUrl(__dirname, 'static/ufo-alert.svg')
@@ -224,6 +226,15 @@ export class Diff extends React.Component<IDiffProps, IDiffState> {
       }
 
       if (this.props.file.status.kind === AppFileStatusKind.Renamed) {
+        // Check if it was changed too
+        if (this.props.file.status.renameIncludesModifications) {
+          return (
+            <div className="panel renamed">
+              <Octicon symbol={OcticonSymbol.alert} />
+              The file was renamed and includes changes.
+            </div>
+          )
+        }
         return (
           <div className="panel renamed">
             The file was renamed but not changed

app/test/unit/git/log-test.ts

@@ -83,6 +83,8 @@ describe('git/log', () => {
       expect(first.files[0].status).toEqual({
         kind: AppFileStatusKind.Renamed,
         oldPath: 'NEW.md',
+        renameIncludesModifications: true,
+        submoduleStatus: undefined,
       })
 
       const second = await getChangedFiles(repository, 'c898ca8')
@@ -92,6 +94,8 @@ describe('git/log', () => {
       expect(second.files[0].status).toEqual({
         kind: AppFileStatusKind.Renamed,
         oldPath: 'OLD.md',
+        renameIncludesModifications: false,
+        submoduleStatus: undefined,
       })
     })
 
@@ -111,12 +115,16 @@ describe('git/log', () => {
       expect(changesetData.files[0].status).toEqual({
         kind: AppFileStatusKind.Copied,
         oldPath: 'initial.md',
+        renameIncludesModifications: false,
+        submoduleStatus: undefined,
       })
 
       expect(changesetData.files[1].path).toBe('duplicate.md')
       expect(changesetData.files[1].status).toEqual({
         kind: AppFileStatusKind.Copied,
         oldPath: 'initial.md',
+        renameIncludesModifications: false,
+        submoduleStatus: undefined,
       })
     })
 

app/test/unit/git/status-test.ts

@@ -247,6 +247,8 @@ describe('git/status', () => {
         expect(files[0].status).toEqual({
           kind: AppFileStatusKind.Renamed,
           oldPath: 'foo',
+          renameIncludesModifications: false,
+          submoduleStatus: undefined,
         })
       })
 
@@ -275,6 +277,8 @@ describe('git/status', () => {
         expect(files[1].status).toEqual({
           kind: AppFileStatusKind.Copied,
           oldPath: 'CONTRIBUTING.md',
+          renameIncludesModifications: false,
+          submoduleStatus: undefined,
         })
       })