Commit f5f4460f authored by Phil Hughes's avatar Phil Hughes

Fixed unified diff lines not displaying inline diffs correctly

This fixes issues with inline diffs and unified diff lines where
they would sometimes display added and deleted inline
instead of in blocks

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/247934
parent 9ed303a6
...@@ -49,8 +49,12 @@ export default { ...@@ -49,8 +49,12 @@ export default {
...mapState({ ...mapState({
projectPath: state => state.diffs.projectPath, projectPath: state => state.diffs.projectPath,
}), }),
...mapGetters('diffs', ['isInlineView', 'isParallelView']), ...mapGetters('diffs', [
...mapGetters('diffs', ['getCommentFormForDiffFile']), 'isInlineView',
'isParallelView',
'getCommentFormForDiffFile',
'diffLines',
]),
...mapGetters(['getNoteableData', 'noteableType', 'getUserData']), ...mapGetters(['getNoteableData', 'noteableType', 'getUserData']),
diffMode() { diffMode() {
return getDiffMode(this.diffFile); return getDiffMode(this.diffFile);
...@@ -115,17 +119,15 @@ export default { ...@@ -115,17 +119,15 @@ export default {
<inline-diff-view <inline-diff-view
v-if="isInlineView" v-if="isInlineView"
:diff-file="diffFile" :diff-file="diffFile"
:diff-lines=" :diff-lines="diffFile.highlighted_diff_lines"
glFeatures.unifiedDiffLines
? diffFile.parallel_diff_lines
: diffFile.highlighted_diff_lines || []
"
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
/> />
<parallel-diff-view <parallel-diff-view
v-else-if="isParallelView" v-else-if="isParallelView"
:diff-file="diffFile" :diff-file="diffFile"
:diff-lines="diffFile.parallel_diff_lines || []" :diff-lines="
glFeatures.unifiedDiffLines ? diffLines(diffFile) : diffFile.parallel_diff_lines || []
"
:help-page-path="helpPagePath" :help-page-path="helpPagePath"
/> />
<gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" /> <gl-loading-icon v-if="diffFile.renderingLines" size="md" class="mt-3" />
......
...@@ -67,9 +67,7 @@ export default { ...@@ -67,9 +67,7 @@ export default {
computed: { computed: {
...mapState({ ...mapState({
diffViewType(state) { diffViewType(state) {
return this.glFeatures.unifiedDiffLines return this.glFeatures.unifiedDiffLines ? INLINE_DIFF_VIEW_TYPE : state.diffs.diffViewType;
? PARALLEL_DIFF_VIEW_TYPE
: state.diffs.diffViewType;
}, },
diffFiles: state => state.diffs.diffFiles, diffFiles: state => state.diffs.diffFiles,
}), }),
......
...@@ -64,99 +64,37 @@ export default { ...@@ -64,99 +64,37 @@ export default {
<col /> <col />
</colgroup> </colgroup>
<tbody> <tbody>
<template v-if="glFeatures.unifiedDiffLines"> <template v-for="(line, index) in diffLines">
<template v-for="({ left, right }, index) in diffLines"> <inline-diff-expansion-row
<inline-diff-expansion-row :key="`expand-${index}`"
:key="`expand-${index}`" :file-hash="diffFile.file_hash"
:file-hash="diffFile.file_hash" :context-lines-path="diffFile.context_lines_path"
:context-lines-path="diffFile.context_lines_path" :line="line"
:line="left || right" :is-top="index === 0"
:is-top="index === 0" :is-bottom="index + 1 === diffLinesLength"
:is-bottom="index + 1 === diffLinesLength" />
/> <inline-diff-table-row
<template v-if="left"> :key="`${line.line_code || index}`"
<inline-diff-table-row :file-hash="diffFile.file_hash"
:key="`${left.line_code || index}`" :file-path="diffFile.file_path"
:file-hash="diffFile.file_hash" :line="line"
:file-path="diffFile.file_path" :is-bottom="index + 1 === diffLinesLength"
:line="left" :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
:is-bottom="index + 1 === diffLinesLength" />
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine" <inline-diff-comment-row
/> :key="`icr-${line.line_code || index}`"
<inline-diff-comment-row :diff-file-hash="diffFile.file_hash"
:key="`icr-${left.line_code || index}`" :line="line"
:diff-file-hash="diffFile.file_hash" :help-page-path="helpPagePath"
:line="left" :has-draft="shouldRenderDraftRow(diffFile.file_hash, line) || false"
:help-page-path="helpPagePath" />
:has-draft="shouldRenderDraftRow(diffFile.file_hash, left) || false" <inline-draft-comment-row
/> v-if="shouldRenderDraftRow(diffFile.file_hash, line)"
<inline-draft-comment-row :key="`draft_${index}`"
v-if="shouldRenderDraftRow(diffFile.file_hash, left)" :draft="draftForLine(diffFile.file_hash, line)"
:key="`draft_${index}`" :diff-file="diffFile"
:draft="draftForLine(diffFile.file_hash, left)" :line="line"
:diff-file="diffFile" />
:line="left"
/>
</template>
<template v-if="right && right.type === 'new'">
<inline-diff-table-row
:key="`new-${right.line_code || index}`"
:file-hash="diffFile.file_hash"
:file-path="diffFile.file_path"
:line="right"
:is-bottom="index + 1 === diffLinesLength"
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
/>
<inline-diff-comment-row
:key="`new-icr-${right.line_code || index}`"
:diff-file-hash="diffFile.file_hash"
:line="right"
:help-page-path="helpPagePath"
:has-draft="shouldRenderDraftRow(diffFile.file_hash, right) || false"
/>
<inline-draft-comment-row
v-if="shouldRenderDraftRow(diffFile.file_hash, right)"
:key="`new-draft_${index}`"
:draft="draftForLine(diffFile.file_hash, right)"
:diff-file="diffFile"
:line="right"
/>
</template>
</template>
</template>
<template v-else>
<template v-for="(line, index) in diffLines">
<inline-diff-expansion-row
:key="`expand-${index}`"
:file-hash="diffFile.file_hash"
:context-lines-path="diffFile.context_lines_path"
:line="line"
:is-top="index === 0"
:is-bottom="index + 1 === diffLinesLength"
/>
<inline-diff-table-row
:key="`${line.line_code || index}`"
:file-hash="diffFile.file_hash"
:file-path="diffFile.file_path"
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
/>
<inline-diff-comment-row
:key="`icr-${line.line_code || index}`"
:diff-file-hash="diffFile.file_hash"
:line="line"
:help-page-path="helpPagePath"
:has-draft="shouldRenderDraftRow(diffFile.file_hash, line) || false"
/>
<inline-draft-comment-row
v-if="shouldRenderDraftRow(diffFile.file_hash, line)"
:key="`draft_${index}`"
:draft="draftForLine(diffFile.file_hash, line)"
:diff-file="diffFile"
:line="line"
/>
</template>
</template> </template>
</tbody> </tbody>
</table> </table>
......
import { __, n__ } from '~/locale'; import { __, n__ } from '~/locale';
import { parallelizeDiffLines } from './utils';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants';
export * from './getters_versions_dropdowns'; export * from './getters_versions_dropdowns';
...@@ -129,3 +130,11 @@ export const fileLineCoverage = state => (file, line) => { ...@@ -129,3 +130,11 @@ export const fileLineCoverage = state => (file, line) => {
*/ */
export const currentDiffIndex = state => export const currentDiffIndex = state =>
Math.max(0, state.diffFiles.findIndex(diff => diff.file_hash === state.currentDiffFileId)); Math.max(0, state.diffFiles.findIndex(diff => diff.file_hash === state.currentDiffFileId));
export const diffLines = state => file => {
if (state.diffViewType === INLINE_DIFF_VIEW_TYPE) {
return null;
}
return parallelizeDiffLines(file.highlighted_diff_lines || []);
};
import Vue from 'vue'; import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { PARALLEL_DIFF_VIEW_TYPE } from '../constants'; import { INLINE_DIFF_VIEW_TYPE } from '../constants';
import { import {
findDiffFile, findDiffFile,
addLineReferences, addLineReferences,
...@@ -152,7 +152,7 @@ export default { ...@@ -152,7 +152,7 @@ export default {
inlineLines: diffFile.highlighted_diff_lines, inlineLines: diffFile.highlighted_diff_lines,
parallelLines: diffFile.parallel_diff_lines, parallelLines: diffFile.parallel_diff_lines,
diffViewType: window.gon?.features?.unifiedDiffLines diffViewType: window.gon?.features?.unifiedDiffLines
? PARALLEL_DIFF_VIEW_TYPE ? INLINE_DIFF_VIEW_TYPE
: state.diffViewType, : state.diffViewType,
contextLines: lines, contextLines: lines,
bottom, bottom,
......
...@@ -350,15 +350,8 @@ function mergeTwoFiles(target, source) { ...@@ -350,15 +350,8 @@ function mergeTwoFiles(target, source) {
} }
function ensureBasicDiffFileLines(file) { function ensureBasicDiffFileLines(file) {
if (window.gon?.features?.unifiedDiffLines) {
return Object.assign(file, {
highlighted_diff_lines: [],
parallel_diff_lines: parallelizeDiffLines(file.highlighted_diff_lines || []),
});
}
const missingInline = !file.highlighted_diff_lines; const missingInline = !file.highlighted_diff_lines;
const missingParallel = !file.parallel_diff_lines; const missingParallel = !file.parallel_diff_lines || window.gon?.features?.unifiedDiffLines;
Object.assign(file, { Object.assign(file, {
highlighted_diff_lines: missingInline ? [] : file.highlighted_diff_lines, highlighted_diff_lines: missingInline ? [] : file.highlighted_diff_lines,
......
...@@ -202,7 +202,7 @@ RSpec.configure do |config| ...@@ -202,7 +202,7 @@ RSpec.configure do |config|
# The following can be removed once we are confident the # The following can be removed once we are confident the
# unified diff lines works as expected # unified diff lines works as expected
stub_feature_flags(unified_diff_lines: false) # stub_feature_flags(unified_diff_lines: false)
# Merge request widget GraphQL requests are disabled in the tests # Merge request widget GraphQL requests are disabled in the tests
# for now whilst we migrate as much as we can over the GraphQL # for now whilst we migrate as much as we can over the GraphQL
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment