Skip to content

G
gitlab-ce
Commit aa84c42f authored by Phil Hughes's avatar Phil Hughes Committed by Igor Drozdov
Browse files
Options

Remove the unified diff lines feature flag 

Removes references to the unified diff lines
feature flag from the diffs app.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/241188
parent dd8012f9
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 217 additions and 927 deletions
app/assets/javascripts/diffs/components/app.vue
View file @ aa84c42f
......@@ -230,9 +230,6 @@ export default {
}
},
diffViewType() {
if (!this.glFeatures.unifiedDiffLines && (this.needsReload() || this.needsFirstLoad())) {
this.refetchDiffData();
}
this.adjustView();
},
shouldShow() {
......
app/assets/javascripts/diffs/components/diff_content.vue
View file @ aa84c42f
......@@ -87,7 +87,7 @@ export default {
return this.getUserData;
},
mappedLines() {
if (this.glFeatures.unifiedDiffLines && this.glFeatures.unifiedDiffComponents) {
if (this.glFeatures.unifiedDiffComponents) {
return this.diffLines(this.diffFile, true).map(mapParallel(this)) || [];
}
......@@ -95,9 +95,7 @@ export default {
if (this.isInlineView) {
return this.diffFile.highlighted_diff_lines.map(mapInline(this));
}
return this.glFeatures.unifiedDiffLines
? this.diffLines(this.diffFile).map(mapParallel(this))
: this.diffFile.parallel_diff_lines.map(mapParallel(this)) || [];
return this.diffLines(this.diffFile).map(mapParallel(this));
},
},
updated() {
......@@ -129,9 +127,7 @@ export default {
<template>
<div class="diff-content">
<div class="diff-viewer">
<template
v-if="isTextFile && glFeatures.unifiedDiffLines && glFeatures.unifiedDiffComponents"
>
<template v-if="isTextFile && glFeatures.unifiedDiffComponents">
<diff-view
:diff-file="diffFile"
:diff-lines="mappedLines"
......
app/assets/javascripts/diffs/components/diff_expansion_cell.vue
View file @ aa84c42f
......@@ -4,7 +4,7 @@ import { GlIcon } from '@gitlab/ui';
import { deprecatedCreateFlash as createFlash } from '~/flash';
import { s__, sprintf } from '~/locale';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { UNFOLD_COUNT, INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '../constants';
import { UNFOLD_COUNT, INLINE_DIFF_VIEW_TYPE, INLINE_DIFF_LINES_KEY } from '../constants';
import * as utils from '../store/utils';
const EXPAND_ALL = 0;
......@@ -14,7 +14,6 @@ const EXPAND_DOWN = 2;
const lineNumberByViewType = (viewType, diffLine) => {
const numberGetters = {
[INLINE_DIFF_VIEW_TYPE]: line => line?.new_line,
[PARALLEL_DIFF_VIEW_TYPE]: line => (line?.right || line?.left)?.new_line,
};
const numberGetter = numberGetters[viewType];
return numberGetter && numberGetter(diffLine);
......@@ -57,9 +56,6 @@ export default {
},
computed: {
...mapState({
diffViewType(state) {
return this.glFeatures.unifiedDiffLines ? INLINE_DIFF_VIEW_TYPE : state.diffs.diffViewType;
},
diffFiles: state => state.diffs.diffFiles,
}),
canExpandUp() {
......@@ -77,16 +73,14 @@ export default {
...mapActions('diffs', ['loadMoreLines']),
getPrevLineNumber(oldLineNumber, newLineNumber) {
const diffFile = utils.findDiffFile(this.diffFiles, this.fileHash);
const lines = {
[INLINE_DIFF_VIEW_TYPE]: diffFile.highlighted_diff_lines,
[PARALLEL_DIFF_VIEW_TYPE]: diffFile.parallel_diff_lines,
};
const index = utils.getPreviousLineIndex(this.diffViewType, diffFile, {
const index = utils.getPreviousLineIndex(INLINE_DIFF_VIEW_TYPE, diffFile, {
oldLineNumber,
newLineNumber,
});
return lineNumberByViewType(this.diffViewType, lines[this.diffViewType][index - 2]) || 0;
return (
lineNumberByViewType(INLINE_DIFF_VIEW_TYPE, diffFile[INLINE_DIFF_LINES_KEY][index - 2]) || 0
);
},
callLoadMoreLines(
endpoint,
......@@ -113,7 +107,7 @@ export default {
this.isRequesting = true;
const endpoint = this.contextLinesPath;
const { fileHash } = this;
const view = this.diffViewType;
const view = INLINE_DIFF_VIEW_TYPE;
const oldLineNumber = this.line.meta_data.old_pos || 0;
const newLineNumber = this.line.meta_data.new_pos || 0;
const offset = newLineNumber - oldLineNumber;
......
app/assets/javascripts/diffs/components/diff_line_note_form.vue
View file @ aa84c42f
......@@ -7,7 +7,7 @@ import noteForm from '../../notes/components/note_form.vue';
import MultilineCommentForm from '../../notes/components/multiline_comment_form.vue';
import autosave from '../../notes/mixins/autosave';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
import { DIFF_NOTE_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '../constants';
import { DIFF_NOTE_TYPE, INLINE_DIFF_LINES_KEY, PARALLEL_DIFF_VIEW_TYPE } from '../constants';
import {
commentLineOptions,
formatLineRange,
......@@ -102,13 +102,13 @@ export default {
};
const getDiffLines = () => {
if (this.diffViewType === PARALLEL_DIFF_VIEW_TYPE) {
return (this.glFeatures.unifiedDiffLines
? this.diffLines(this.diffFile)
: this.diffFile.parallel_diff_lines
).reduce(combineSides, []);
return this.diffLines(this.diffFile, this.glFeatures.unifiedDiffComponents).reduce(
combineSides,
[],
);
}
return this.diffFile.highlighted_diff_lines;
return this.diffFile[INLINE_DIFF_LINES_KEY];
};
const side = this.line.type === 'new' ? 'right' : 'left';
const lines = getDiffLines();
......
app/assets/javascripts/diffs/store/actions.js
View file @ aa84c42f
......@@ -30,13 +30,11 @@ import {
OLD_LINE_KEY,
NEW_LINE_KEY,
TYPE_KEY,
LEFT_LINE_KEY,
MAX_RENDERING_DIFF_LINES,
MAX_RENDERING_BULK_ROWS,
MIN_RENDERING_MS,
START_RENDERING_INDEX,
INLINE_DIFF_LINES_KEY,
PARALLEL_DIFF_LINES_KEY,
DIFFS_PER_PAGE,
DIFF_WHITESPACE_COOKIE_NAME,
SHOW_WHITESPACE,
......@@ -77,7 +75,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
const urlParams = {
per_page: DIFFS_PER_PAGE,
w: state.showWhitespace ? '0' : '1',
view: window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType,
view: 'inline',
};
commit(types.SET_BATCH_LOADING, true);
......@@ -140,7 +138,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
export const fetchDiffFilesMeta = ({ commit, state }) => {
const worker = new TreeWorker();
const urlParams = {
view: window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType,
view: 'inline',
};
commit(types.SET_LOADING, true);
......@@ -401,15 +399,10 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => {
export const toggleFileDiscussionWrappers = ({ commit }, diff) => {
const discussionWrappersExpanded = allDiscussionWrappersExpanded(diff);
const lineCodesWithDiscussions = new Set();
const { parallel_diff_lines: parallelLines, highlighted_diff_lines: inlineLines } = diff;
const allLines = inlineLines.concat(
parallelLines.map(line => line.left),
parallelLines.map(line => line.right),
);
const lineHasDiscussion = line => Boolean(line?.discussions.length);
const registerDiscussionLine = line => lineCodesWithDiscussions.add(line.line_code);
allLines.filter(lineHasDiscussion).forEach(registerDiscussionLine);
diff[INLINE_DIFF_LINES_KEY].filter(lineHasDiscussion).forEach(registerDiscussionLine);
if (lineCodesWithDiscussions.size) {
Array.from(lineCodesWithDiscussions).forEach(lineCode => {
......@@ -508,61 +501,26 @@ export const receiveFullDiffError = ({ commit }, filePath) => {
createFlash(s__('MergeRequest|Error loading full diff. Please try again.'));
};
export const setExpandedDiffLines = ({ commit, state }, { file, data }) => {
const expandedDiffLines = {
highlighted_diff_lines: convertExpandLines({
diffLines: file.highlighted_diff_lines,
typeKey: TYPE_KEY,
oldLineKey: OLD_LINE_KEY,
newLineKey: NEW_LINE_KEY,
data,
mapLine: ({ line, oldLine, newLine }) =>
Object.assign(line, {
old_line: oldLine,
new_line: newLine,
line_code: `${file.file_hash}_${oldLine}_${newLine}`,
}),
}),
parallel_diff_lines: convertExpandLines({
diffLines: file.parallel_diff_lines,
typeKey: [LEFT_LINE_KEY, TYPE_KEY],
oldLineKey: [LEFT_LINE_KEY, OLD_LINE_KEY],
newLineKey: [LEFT_LINE_KEY, NEW_LINE_KEY],
data,
mapLine: ({ line, oldLine, newLine }) => ({
left: {
...line,
old_line: oldLine,
line_code: `${file.file_hash}_${oldLine}_${newLine}`,
},
right: {
...line,
new_line: newLine,
line_code: `${file.file_hash}_${newLine}_${oldLine}`,
},
export const setExpandedDiffLines = ({ commit }, { file, data }) => {
const expandedDiffLines = convertExpandLines({
diffLines: file[INLINE_DIFF_LINES_KEY],
typeKey: TYPE_KEY,
oldLineKey: OLD_LINE_KEY,
newLineKey: NEW_LINE_KEY,
data,
mapLine: ({ line, oldLine, newLine }) =>
Object.assign(line, {
old_line: oldLine,
new_line: newLine,
line_code: `${file.file_hash}_${oldLine}_${newLine}`,
}),
}),
};
const unifiedDiffLinesEnabled = window.gon?.features?.unifiedDiffLines;
const currentDiffLinesKey =
state.diffViewType === INLINE_DIFF_VIEW_TYPE || unifiedDiffLinesEnabled
? INLINE_DIFF_LINES_KEY
: PARALLEL_DIFF_LINES_KEY;
const hiddenDiffLinesKey =
state.diffViewType === INLINE_DIFF_VIEW_TYPE ? PARALLEL_DIFF_LINES_KEY : INLINE_DIFF_LINES_KEY;
if (!unifiedDiffLinesEnabled) {
commit(types.SET_HIDDEN_VIEW_DIFF_FILE_LINES, {
filePath: file.file_path,
lines: expandedDiffLines[hiddenDiffLinesKey],
});
}
});
if (expandedDiffLines[currentDiffLinesKey].length > MAX_RENDERING_DIFF_LINES) {
if (expandedDiffLines.length > MAX_RENDERING_DIFF_LINES) {
let index = START_RENDERING_INDEX;
commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, {
filePath: file.file_path,
lines: expandedDiffLines[currentDiffLinesKey].slice(0, index),
lines: expandedDiffLines.slice(0, index),
});
commit(types.TOGGLE_DIFF_FILE_RENDERING_MORE, file.file_path);
......@@ -571,10 +529,10 @@ export const setExpandedDiffLines = ({ commit, state }, { file, data }) => {
while (
t.timeRemaining() >= MIN_RENDERING_MS &&
index !== expandedDiffLines[currentDiffLinesKey].length &&
index !== expandedDiffLines.length &&
index - startIndex !== MAX_RENDERING_BULK_ROWS
) {
const line = expandedDiffLines[currentDiffLinesKey][index];
const line = expandedDiffLines[index];
if (line) {
commit(types.ADD_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: file.file_path, line });
......@@ -582,7 +540,7 @@ export const setExpandedDiffLines = ({ commit, state }, { file, data }) => {
}
}
if (index !== expandedDiffLines[currentDiffLinesKey].length) {
if (index !== expandedDiffLines.length) {
idleCallback(idleCb);
} else {
commit(types.TOGGLE_DIFF_FILE_RENDERING_MORE, file.file_path);
......@@ -593,7 +551,7 @@ export const setExpandedDiffLines = ({ commit, state }, { file, data }) => {
} else {
commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, {
filePath: file.file_path,
lines: expandedDiffLines[currentDiffLinesKey],
lines: expandedDiffLines,
});
}
};
......@@ -627,7 +585,7 @@ export const toggleFullDiff = ({ dispatch, commit, getters, state }, filePath) =
}
};
export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { diffFile }) {
export function switchToFullDiffFromRenamedFile({ commit, dispatch }, { diffFile }) {
return axios
.get(diffFile.context_lines_path, {
params: {
......@@ -638,7 +596,7 @@ export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { d
.then(({ data }) => {
const lines = data.map((line, index) =>
prepareLineForRenamedFile({
diffViewType: window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType,
diffViewType: 'inline',
line,
diffFile,
index,
......
app/assets/javascripts/diffs/store/getters.js
View file @ aa84c42f
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,
INLINE_DIFF_LINES_KEY,
} from '../constants';
export * from './getters_versions_dropdowns';
......@@ -54,24 +58,10 @@ export const diffHasAllCollapsedDiscussions = (state, getters) => diff => {
* @param {Object} diff
* @returns {Boolean}
*/
export const diffHasExpandedDiscussions = state => diff => {
const lines = {
[INLINE_DIFF_VIEW_TYPE]: diff.highlighted_diff_lines || [],
[PARALLEL_DIFF_VIEW_TYPE]: (diff.parallel_diff_lines || []).reduce((acc, line) => {
if (line.left) {
acc.push(line.left);
}
if (line.right) {
acc.push(line.right);
}
return acc;
}, []),
};
return lines[window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType]
.filter(l => l.discussions.length >= 1)
.some(l => l.discussionsExpanded);
export const diffHasExpandedDiscussions = () => diff => {
return diff[INLINE_DIFF_LINES_KEY].filter(l => l.discussions.length >= 1).some(
l => l.discussionsExpanded,
);
};
/**
......@@ -79,24 +69,8 @@ export const diffHasExpandedDiscussions = state => diff => {
* @param {Boolean} diff
* @returns {Boolean}
*/
export const diffHasDiscussions = state => diff => {
const lines = {
[INLINE_DIFF_VIEW_TYPE]: diff.highlighted_diff_lines || [],
[PARALLEL_DIFF_VIEW_TYPE]: (diff.parallel_diff_lines || []).reduce((acc, line) => {
if (line.left) {
acc.push(line.left);
}
if (line.right) {
acc.push(line.right);
}
return acc;
}, []),
};
return lines[window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType].some(
l => l.discussions.length >= 1,
);
export const diffHasDiscussions = () => diff => {
return diff[INLINE_DIFF_LINES_KEY].some(l => l.discussions.length >= 1);
};
/**
......
app/assets/javascripts/diffs/store/mutation_types.js
View file @ aa84c42f
......@@ -35,7 +35,6 @@ export const RECEIVE_FULL_DIFF_SUCCESS = 'RECEIVE_FULL_DIFF_SUCCESS';
export const RECEIVE_FULL_DIFF_ERROR = 'RECEIVE_FULL_DIFF_ERROR';
export const SET_FILE_COLLAPSED = 'SET_FILE_COLLAPSED';
export const SET_HIDDEN_VIEW_DIFF_FILE_LINES = 'SET_HIDDEN_VIEW_DIFF_FILE_LINES';
export const SET_CURRENT_VIEW_DIFF_FILE_LINES = 'SET_CURRENT_VIEW_DIFF_FILE_LINES';
export const ADD_CURRENT_VIEW_DIFF_FILE_LINES = 'ADD_CURRENT_VIEW_DIFF_FILE_LINES';
export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE';
......
app/assets/javascripts/diffs/store/mutations.js
View file @ aa84c42f
import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import {
DIFF_FILE_MANUAL_COLLAPSE,
DIFF_FILE_AUTOMATIC_COLLAPSE,
INLINE_DIFF_VIEW_TYPE,
} from '../constants';
import {
findDiffFile,
addLineReferences,
......@@ -14,6 +9,11 @@ import {
isDiscussionApplicableToLine,
updateLineInFile,
} from './utils';
import {
DIFF_FILE_MANUAL_COLLAPSE,
DIFF_FILE_AUTOMATIC_COLLAPSE,
INLINE_DIFF_LINES_KEY,
} from '../constants';
import * as types from './mutation_types';
function updateDiffFilesInState(state, files) {
......@@ -109,25 +109,7 @@ export default {
if (!diffFile) return;
if (diffFile.highlighted_diff_lines.length) {
diffFile.highlighted_diff_lines.find(l => l.line_code === lineCode).hasForm = hasForm;
}
if (diffFile.parallel_diff_lines.length) {
const line = diffFile.parallel_diff_lines.find(l => {
const { left, right } = l;
return (left && left.line_code === lineCode) || (right && right.line_code === lineCode);
});
if (line.left && line.left.line_code === lineCode) {
line.left.hasForm = hasForm;
}
if (line.right && line.right.line_code === lineCode) {
line.right.hasForm = hasForm;
}
}
diffFile[INLINE_DIFF_LINES_KEY].find(l => l.line_code === lineCode).hasForm = hasForm;
},
[types.ADD_CONTEXT_LINES](state, options) {
......@@ -157,11 +139,7 @@ export default {
});
addContextLines({
inlineLines: diffFile.highlighted_diff_lines,
parallelLines: diffFile.parallel_diff_lines,
diffViewType: window.gon?.features?.unifiedDiffLines
? INLINE_DIFF_VIEW_TYPE
: state.diffViewType,
inlineLines: diffFile[INLINE_DIFF_LINES_KEY],
contextLines: lines,
bottom,
lineNumbers,
......@@ -219,8 +197,8 @@ export default {
state.diffFiles.forEach(file => {
if (file.file_hash === fileHash) {
if (file.highlighted_diff_lines.length) {
file.highlighted_diff_lines.forEach(line => {
if (file[INLINE_DIFF_LINES_KEY].length) {
file[INLINE_DIFF_LINES_KEY].forEach(line => {
Object.assign(
line,
setDiscussionsExpanded(lineCheck(line) ? mapDiscussions(line) : line),
......@@ -228,25 +206,7 @@ export default {
});
}
if (file.parallel_diff_lines.length) {
file.parallel_diff_lines.forEach(line => {
const left = line.left && lineCheck(line.left);
const right = line.right && lineCheck(line.right);
if (left || right) {
Object.assign(line, {
left: line.left ? setDiscussionsExpanded(mapDiscussions(line.left)) : null,
right: line.right
? setDiscussionsExpanded(mapDiscussions(line.right, () => !left))
: null,
});
}
return line;
});
}
if (!file.parallel_diff_lines.length || !file.highlighted_diff_lines.length) {
if (!file[INLINE_DIFF_LINES_KEY].length) {
const newDiscussions = (file.discussions || [])
.filter(d => d.id !== discussion.id)
.concat(discussion);
......@@ -369,31 +329,15 @@ export default {
renderFile(file);
}
},
[types.SET_HIDDEN_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
const file = state.diffFiles.find(f => f.file_path === filePath);
const hiddenDiffLinesKey =
state.diffViewType === 'inline' ? 'parallel_diff_lines' : 'highlighted_diff_lines';
file[hiddenDiffLinesKey] = lines;
},
[types.SET_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, lines }) {
const file = state.diffFiles.find(f => f.file_path === filePath);
let currentDiffLinesKey;
if (window.gon?.features?.unifiedDiffLines || state.diffViewType === 'inline') {
currentDiffLinesKey = 'highlighted_diff_lines';
} else {
currentDiffLinesKey = 'parallel_diff_lines';
}
file[currentDiffLinesKey] = lines;
file[INLINE_DIFF_LINES_KEY] = lines;
},
[types.ADD_CURRENT_VIEW_DIFF_FILE_LINES](state, { filePath, line }) {
const file = state.diffFiles.find(f => f.file_path === filePath);
const currentDiffLinesKey =
state.diffViewType === 'inline' ? 'highlighted_diff_lines' : 'parallel_diff_lines';
file[currentDiffLinesKey].push(line);
file[INLINE_DIFF_LINES_KEY].push(line);
},
[types.TOGGLE_DIFF_FILE_RENDERING_MORE](state, filePath) {
const file = state.diffFiles.find(f => f.file_path === filePath);
......
app/assets/javascripts/diffs/store/utils.js
View file @ aa84c42f
......@@ -12,8 +12,7 @@ import {
MATCH_LINE_TYPE,
LINES_TO_BE_RENDERED_DIRECTLY,
TREE_TYPE,
INLINE_DIFF_VIEW_TYPE,
PARALLEL_DIFF_VIEW_TYPE,
INLINE_DIFF_LINES_KEY,
SHOW_WHITESPACE,
NO_SHOW_WHITESPACE,
} from '../constants';
......@@ -178,43 +177,16 @@ export const findIndexInInlineLines = (lines, lineNumbers) => {
);
};
export const findIndexInParallelLines = (lines, lineNumbers) => {
const { oldLineNumber, newLineNumber } = lineNumbers;
return lines.findIndex(
line =>
line.left &&
line.right &&
line.left.old_line === oldLineNumber &&
line.right.new_line === newLineNumber,
);
};
const indexGettersByViewType = {
[INLINE_DIFF_VIEW_TYPE]: findIndexInInlineLines,
[PARALLEL_DIFF_VIEW_TYPE]: findIndexInParallelLines,
};
export const getPreviousLineIndex = (diffViewType, file, lineNumbers) => {
const findIndex = indexGettersByViewType[diffViewType];
const lines = {
[INLINE_DIFF_VIEW_TYPE]: file.highlighted_diff_lines,
[PARALLEL_DIFF_VIEW_TYPE]: file.parallel_diff_lines,
};
return findIndex && findIndex(lines[diffViewType], lineNumbers);
return findIndexInInlineLines(file[INLINE_DIFF_LINES_KEY], lineNumbers);
};
export function removeMatchLine(diffFile, lineNumbers, bottom) {
const indexForInline = findIndexInInlineLines(diffFile.highlighted_diff_lines, lineNumbers);
const indexForParallel = findIndexInParallelLines(diffFile.parallel_diff_lines, lineNumbers);
const indexForInline = findIndexInInlineLines(diffFile[INLINE_DIFF_LINES_KEY], lineNumbers);
const factor = bottom ? 1 : -1;
if (indexForInline > -1) {
diffFile.highlighted_diff_lines.splice(indexForInline + factor, 1);
}
if (indexForParallel > -1) {
diffFile.parallel_diff_lines.splice(indexForParallel + factor, 1);
diffFile[INLINE_DIFF_LINES_KEY].splice(indexForInline + factor, 1);
}
}
......@@ -257,24 +229,6 @@ export function addLineReferences(lines, lineNumbers, bottom, isExpandDown, next
return linesWithNumbers;
}
function addParallelContextLines(options) {
const { parallelLines, contextLines, lineNumbers, isExpandDown } = options;
const normalizedParallelLines = contextLines.map(line => ({
left: line,
right: line,
line_code: line.line_code,
}));
const factor = isExpandDown ? 1 : 0;
if (!isExpandDown && options.bottom) {
parallelLines.push(...normalizedParallelLines);
} else {
const parallelIndex = findIndexInParallelLines(parallelLines, lineNumbers);
parallelLines.splice(parallelIndex + factor, 0, ...normalizedParallelLines);
}
}
function addInlineContextLines(options) {
const { inlineLines, contextLines, lineNumbers, isExpandDown } = options;
const factor = isExpandDown ? 1 : 0;
......@@ -289,16 +243,7 @@ function addInlineContextLines(options) {
}
export function addContextLines(options) {
const { diffViewType } = options;
const contextLineHandlers = {
[INLINE_DIFF_VIEW_TYPE]: addInlineContextLines,
[PARALLEL_DIFF_VIEW_TYPE]: addParallelContextLines,
};
const contextLineHandler = contextLineHandlers[diffViewType];
if (contextLineHandler) {
contextLineHandler(options);
}
addInlineContextLines(options);
}
/**
......@@ -324,41 +269,29 @@ export function trimFirstCharOfLineContent(line = {}) {
return parsedLine;
}
function getLineCode({ left, right }, index) {
if (left && left.line_code) {
return left.line_code;
} else if (right && right.line_code) {
return right.line_code;
}
return index;
}
function diffFileUniqueId(file) {
return `${file.content_sha}-${file.file_hash}`;
}
function mergeTwoFiles(target, source) {
const originalInline = target.highlighted_diff_lines;
const originalParallel = target.parallel_diff_lines;
const originalInline = target[INLINE_DIFF_LINES_KEY];
const missingInline = !originalInline.length;
const missingParallel = !originalParallel.length;
return {
...target,
highlighted_diff_lines: missingInline ? source.highlighted_diff_lines : originalInline,
parallel_diff_lines: missingParallel ? source.parallel_diff_lines : originalParallel,
[INLINE_DIFF_LINES_KEY]: missingInline ? source[INLINE_DIFF_LINES_KEY] : originalInline,
parallel_diff_lines: null,
renderIt: source.renderIt,
collapsed: source.collapsed,
};
}
function ensureBasicDiffFileLines(file) {
const missingInline = !file.highlighted_diff_lines;
const missingParallel = !file.parallel_diff_lines || window.gon?.features?.unifiedDiffLines;
const missingInline = !file[INLINE_DIFF_LINES_KEY];
Object.assign(file, {
highlighted_diff_lines: missingInline ? [] : file.highlighted_diff_lines,
parallel_diff_lines: missingParallel ? [] : file.parallel_diff_lines,
[INLINE_DIFF_LINES_KEY]: missingInline ? [] : file[INLINE_DIFF_LINES_KEY],
parallel_diff_lines: null,
});
return file;
......@@ -382,7 +315,7 @@ function prepareLine(line, file) {
}
}
export function prepareLineForRenamedFile({ line, diffViewType, diffFile, index = 0 }) {
export function prepareLineForRenamedFile({ line, diffFile, index = 0 }) {
/*
Renamed files are a little different than other diffs, which
is why this is distinct from `prepareDiffFileLines` below.
......@@ -407,48 +340,23 @@ export function prepareLineForRenamedFile({ line, diffViewType, diffFile, index
prepareLine(cleanLine, diffFile); // WARNING: In-Place Mutations!
if (diffViewType === PARALLEL_DIFF_VIEW_TYPE) {
return {
left: { ...cleanLine },
right: { ...cleanLine },
line_code: cleanLine.line_code,
};
}
return cleanLine;
}
function prepareDiffFileLines(file) {
const inlineLines = file.highlighted_diff_lines;
const parallelLines = file.parallel_diff_lines;
let parallelLinesCount = 0;
const inlineLines = file[INLINE_DIFF_LINES_KEY];
inlineLines.forEach(line => prepareLine(line, file)); // WARNING: In-Place Mutations!
parallelLines.forEach((line, index) => {
Object.assign(line, { line_code: getLineCode(line, index) });
if (line.left) {
parallelLinesCount += 1;
prepareLine(line.left, file); // WARNING: In-Place Mutations!
}
if (line.right) {
parallelLinesCount += 1;
prepareLine(line.right, file); // WARNING: In-Place Mutations!
}
});
Object.assign(file, {
inlineLinesCount: inlineLines.length,
parallelLinesCount,
});
return file;
}
function getVisibleDiffLines(file) {
return Math.max(file.inlineLinesCount, file.parallelLinesCount);
return file.inlineLinesCount;
}
function finalizeDiffFile(file) {
......@@ -490,43 +398,14 @@ export function prepareDiffData(diff, priorFiles = []) {
export function getDiffPositionByLineCode(diffFiles) {
let lines = [];
const hasInlineDiffs = diffFiles.some(file => file.highlighted_diff_lines.length > 0);
if (hasInlineDiffs) {
// In either of these cases, we can use `highlighted_diff_lines` because
// that will include all of the parallel diff lines, too
lines = diffFiles.reduce((acc, diffFile) => {
diffFile.highlighted_diff_lines.forEach(line => {
acc.push({ file: diffFile, line });
});
return acc;
}, []);
} else {
// If we're in single diff view mode and the inline lines haven't been
// loaded yet, we need to parse the parallel lines
lines = diffFiles.reduce((acc, diffFile) => {
diffFile.parallel_diff_lines.forEach(pair => {
// It's possible for a parallel line to have an opposite line that doesn't exist
// For example: *deleted* lines will have `null` right lines, while
// *added* lines will have `null` left lines.
// So we have to check each line before we push it onto the array so we're not
// pushing null line diffs
if (pair.left) {
acc.push({ file: diffFile, line: pair.left });
}
if (pair.right) {
acc.push({ file: diffFile, line: pair.right });
}
});
lines = diffFiles.reduce((acc, diffFile) => {
diffFile[INLINE_DIFF_LINES_KEY].forEach(line => {
acc.push({ file: diffFile, line });
});
return acc;
}, []);
}
return acc;
}, []);
return lines.reduce((acc, { file, line }) => {
if (line.line_code) {
......@@ -739,24 +618,10 @@ export const convertExpandLines = ({
export const idleCallback = cb => requestIdleCallback(cb);
function getLinesFromFileByLineCode(file, lineCode) {
const parallelLines = file.parallel_diff_lines;
const inlineLines = file.highlighted_diff_lines;
const inlineLines = file[INLINE_DIFF_LINES_KEY];
const matchesCode = line => line.line_code === lineCode;
return [
...parallelLines.reduce((acc, line) => {
if (line.left) {
acc.push(line.left);
}
if (line.right) {
acc.push(line.right);
}
return acc;
}, []),
...inlineLines,
].filter(matchesCode);
return inlineLines.filter(matchesCode);
}
export const updateLineInFile = (selectedFile, lineCode, updateFn) => {
......@@ -771,12 +636,7 @@ export const allDiscussionWrappersExpanded = diff => {
}
};
diff.parallel_diff_lines.forEach(line => {
changeExpandedResult(line.left);
changeExpandedResult(line.right);
});
diff.highlighted_diff_lines.forEach(line => {
diff[INLINE_DIFF_LINES_KEY].forEach(line => {
changeExpandedResult(line);
});
......
app/assets/javascripts/notes/components/noteable_note.vue
View file @ aa84c42f
......@@ -23,6 +23,7 @@ import {
commentLineOptions,
formatLineRange,
} from './multiline_comment_utils';
import { INLINE_DIFF_LINES_KEY } from '~/diffs/constants';
export default {
name: 'NoteableNote',
......@@ -169,12 +170,8 @@ export default {
return this.line && this.startLineNumber !== this.endLineNumber;
},
commentLineOptions() {
const sideA = this.line.type === 'new' ? 'right' : 'left';
const sideB = sideA === 'left' ? 'right' : 'left';
const lines = this.diffFile.highlighted_diff_lines.length
? this.diffFile.highlighted_diff_lines
: this.diffFile.parallel_diff_lines.map(l => l[sideA] || l[sideB]);
return commentLineOptions(lines, this.commentLineStart, this.line.line_code, sideA);
const lines = this.diffFile[INLINE_DIFF_LINES_KEY].length;
return commentLineOptions(lines, this.commentLineStart, this.line.line_code);
},
diffFile() {
if (this.commentLineStart.line_code) {
......
app/controllers/projects/merge_requests/diffs_controller.rb
View file @ aa84c42f
......@@ -69,7 +69,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
}
options = additional_attributes.merge(
diff_view: unified_diff_lines_view_type(@merge_request.project),
diff_view: "inline",
merge_ref_head_diff: render_merge_ref_head_diff?
)
......
app/controllers/projects/merge_requests_controller.rb
View file @ aa84c42f
......@@ -36,7 +36,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:approvals_commented_by, @project, default_enabled: true)
push_frontend_feature_flag(:hide_jump_to_next_unresolved_in_threads, default_enabled: true)
push_frontend_feature_flag(:merge_request_widget_graphql, @project)
push_frontend_feature_flag(:unified_diff_lines, @project, default_enabled: true)
push_frontend_feature_flag(:unified_diff_components, @project)
push_frontend_feature_flag(:highlight_current_diff_row, @project)
push_frontend_feature_flag(:default_merge_ref_for_diffs, @project)
......@@ -481,7 +480,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
def endpoint_metadata_url(project, merge_request)
params = request.query_parameters
params[:view] = unified_diff_lines_view_type(project)
params[:view] = "inline"
if Feature.enabled?(:default_merge_ref_for_diffs, project)
params = params.merge(diff_head: true)
......
app/helpers/diff_helper.rb
View file @ aa84c42f
......@@ -203,14 +203,6 @@ module DiffHelper
set_secure_cookie(:diff_view, params.delete(:view), type: CookiesHelper::COOKIE_TYPE_PERMANENT) if params[:view].present?
end
def unified_diff_lines_view_type(project)
if Feature.enabled?(:unified_diff_lines, project, default_enabled: true)
'inline'
else
diff_view
end
end
private
def diff_btn(title, name, selected)
......
config/feature_flags/development/unified_diff_lines.yml deleted 100644 → 0
View file @ dd8012f9
---
name: unified_diff_lines
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40131
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/241188
milestone: '13.4'
type: development
group: group::source code
default_enabled: true
spec/frontend/diffs/components/app_spec.js
View file @ aa84c42f
......@@ -12,7 +12,6 @@ import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue';
import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
import CommitWidget from '~/diffs/components/commit_widget.vue';
import TreeList from '~/diffs/components/tree_list.vue';
import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '~/diffs/constants';
import createDiffsStore from '../create_diffs_store';
import axios from '~/lib/utils/axios_utils';
import * as urlUtils from '~/lib/utils/url_utility';
......@@ -75,12 +74,6 @@ describe('diffs/components/app', () => {
});
}
function getOppositeViewType(currentViewType) {
return currentViewType === INLINE_DIFF_VIEW_TYPE
? PARALLEL_DIFF_VIEW_TYPE
: INLINE_DIFF_VIEW_TYPE;
}
beforeEach(() => {
// setup globals (needed for component to mount :/)
window.mrTabs = {
......@@ -125,104 +118,6 @@ describe('diffs/components/app', () => {
wrapper.vm.$nextTick(done);
});
describe('when the diff view type changes and it should load a single diff view style', () => {
const noLinesDiff = {
highlighted_diff_lines: [],
parallel_diff_lines: [],
};
const parallelLinesDiff = {
highlighted_diff_lines: [],
parallel_diff_lines: ['line'],
};
const inlineLinesDiff = {
highlighted_diff_lines: ['line'],
parallel_diff_lines: [],
};
const fullDiff = {
highlighted_diff_lines: ['line'],
parallel_diff_lines: ['line'],
};
function expectFetchToOccur({ vueInstance, done = () => {}, existingFiles = 1 } = {}) {
vueInstance.$nextTick(() => {
expect(vueInstance.diffFiles.length).toEqual(existingFiles);
expect(vueInstance.fetchDiffFilesBatch).toHaveBeenCalled();
done();
});
}
it('fetches diffs if it has none', done => {
wrapper.vm.isLatestVersion = () => false;
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, existingFiles: 0, done });
});
it('fetches diffs if it has both view styles, but no lines in either', done => {
wrapper.vm.isLatestVersion = () => false;
store.state.diffs.diffFiles.push(noLinesDiff);
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, done });
});
it('fetches diffs if it only has inline view style', done => {
wrapper.vm.isLatestVersion = () => false;
store.state.diffs.diffFiles.push(inlineLinesDiff);
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, done });
});
it('fetches diffs if it only has parallel view style', done => {
wrapper.vm.isLatestVersion = () => false;
store.state.diffs.diffFiles.push(parallelLinesDiff);
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, done });
});
it('fetches batch diffs if it has none', done => {
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, existingFiles: 0, done });
});
it('fetches batch diffs if it has both view styles, but no lines in either', done => {
store.state.diffs.diffFiles.push(noLinesDiff);
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, done });
});
it('fetches batch diffs if it only has inline view style', done => {
store.state.diffs.diffFiles.push(inlineLinesDiff);
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, done });
});
it('fetches batch diffs if it only has parallel view style', done => {
store.state.diffs.diffFiles.push(parallelLinesDiff);
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expectFetchToOccur({ vueInstance: wrapper.vm, done });
});
it('does not fetch batch diffs if it has already fetched both styles of diff', () => {
store.state.diffs.diffFiles.push(fullDiff);
store.state.diffs.diffViewType = getOppositeViewType(wrapper.vm.diffViewType);
expect(wrapper.vm.diffFiles.length).toEqual(1);
expect(wrapper.vm.fetchDiffFilesBatch).not.toHaveBeenCalled();
});
});
it('calls batch methods if diffsBatchLoad is enabled, and not latest version', done => {
expect(wrapper.vm.diffFilesLength).toEqual(0);
wrapper.vm.isLatestVersion = () => false;
......
spec/frontend/diffs/components/diff_content_spec.js
View file @ aa84c42f
......@@ -12,7 +12,6 @@ import DiffDiscussions from '~/diffs/components/diff_discussions.vue';
import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants';
import diffFileMockData from '../mock_data/diff_file';
import { diffViewerModes } from '~/ide/constants';
import { diffLines } from '~/diffs/store/getters';
import DiffView from '~/diffs/components/diff_view.vue';
const localVue = createLocalVue();
......@@ -74,7 +73,7 @@ describe('DiffContent', () => {
isInlineView: isInlineViewGetterMock,
isParallelView: isParallelViewGetterMock,
getCommentFormForDiffFile: getCommentFormForDiffFileGetterMock,
diffLines,
diffLines: () => () => [...diffFileMockData.parallel_diff_lines],
},
actions: {
saveDiffDiscussion: saveDiffDiscussionMock,
......@@ -122,11 +121,11 @@ describe('DiffContent', () => {
expect(wrapper.find(ParallelDiffView).exists()).toBe(true);
});
it('should render diff view if `unifiedDiffLines` & `unifiedDiffComponents` are true', () => {
it('should render diff view if `unifiedDiffComponents` are true', () => {
isParallelViewGetterMock.mockReturnValue(true);
createComponent({
props: { diffFile: textDiffFile },
provide: { glFeatures: { unifiedDiffLines: true, unifiedDiffComponents: true } },
provide: { glFeatures: { unifiedDiffComponents: true } },
});
expect(wrapper.find(DiffView).exists()).toBe(true);
......
spec/frontend/diffs/components/diff_expansion_cell_spec.js
View file @ aa84c42f
......@@ -5,18 +5,16 @@ import { getByText } from '@testing-library/dom';
import { createStore } from '~/mr_notes/stores';
import DiffExpansionCell from '~/diffs/components/diff_expansion_cell.vue';
import { getPreviousLineIndex } from '~/diffs/store/utils';
import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '~/diffs/constants';
import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
import diffFileMockData from '../mock_data/diff_file';
const EXPAND_UP_CLASS = '.js-unfold';
const EXPAND_DOWN_CLASS = '.js-unfold-down';
const lineSources = {
[INLINE_DIFF_VIEW_TYPE]: 'highlighted_diff_lines',
[PARALLEL_DIFF_VIEW_TYPE]: 'parallel_diff_lines',
};
const lineHandlers = {
[INLINE_DIFF_VIEW_TYPE]: line => line,
[PARALLEL_DIFF_VIEW_TYPE]: line => line.right || line.left,
};
function makeLoadMoreLinesPayload({
......@@ -126,7 +124,6 @@ describe('DiffExpansionCell', () => {
describe('any row', () => {
[
{ diffViewType: INLINE_DIFF_VIEW_TYPE, lineIndex: 8, file: { parallel_diff_lines: [] } },
{ diffViewType: PARALLEL_DIFF_VIEW_TYPE, lineIndex: 7, file: { highlighted_diff_lines: [] } },
].forEach(({ diffViewType, file, lineIndex }) => {
describe(`with diffViewType (${diffViewType})`, () => {
beforeEach(() => {
......
spec/frontend/diffs/store/actions_spec.js
View file @ aa84c42f
......@@ -1235,10 +1235,6 @@ describe('DiffsStoreActions', () => {
{ file: { file_path: 'path' }, data: [] },
{ diffViewType: 'inline' },
[
{
type: 'SET_HIDDEN_VIEW_DIFF_FILE_LINES',
payload: { filePath: 'path', lines: ['test'] },
},
{
type: 'SET_CURRENT_VIEW_DIFF_FILE_LINES',
payload: { filePath: 'path', lines: ['test'] },
......@@ -1258,10 +1254,6 @@ describe('DiffsStoreActions', () => {
{ file: { file_path: 'path' }, data: [] },
{ diffViewType: 'inline' },
[
{
type: 'SET_HIDDEN_VIEW_DIFF_FILE_LINES',
payload: { filePath: 'path', lines },
},
{
type: 'SET_CURRENT_VIEW_DIFF_FILE_LINES',
payload: { filePath: 'path', lines: lines.slice(0, 200) },
......
spec/frontend/diffs/store/mutations_spec.js
View file @ aa84c42f
This diff is collapsed.
spec/frontend/diffs/store/utils_spec.js
View file @ aa84c42f
This diff is collapsed.
spec/helpers/diff_helper_spec.rb
View file @ aa84c42f
......@@ -358,30 +358,4 @@ RSpec.describe DiffHelper do
expect(diff_file_path_text(diff_file, max: 10)).to eq("...open.rb")
end
end
describe 'unified_diff_lines_view_type' do
before do
controller.params[:view] = 'parallel'
end
describe 'unified diffs enabled' do
before do
stub_feature_flags(unified_diff_lines: true)
end
it 'returns inline view' do
expect(helper.unified_diff_lines_view_type(project)).to eq 'inline'
end
end
describe 'unified diffs disabled' do
before do
stub_feature_flags(unified_diff_lines: false)
end
it 'returns parallel view' do
expect(helper.unified_diff_lines_view_type(project)).to eq :parallel
end
end
end
end
spec/spec_helper.rb
View file @ aa84c42f
......@@ -215,10 +215,6 @@ RSpec.configure do |config|
stub_feature_flags(vue_issuable_sidebar: false)
stub_feature_flags(vue_issuable_epic_sidebar: false)
# The following can be removed once we are confident the
# unified diff lines works as expected
stub_feature_flags(unified_diff_lines: false)
# Merge request widget GraphQL requests are disabled in the tests
# for now whilst we migrate as much as we can over the GraphQL
stub_feature_flags(merge_request_widget_graphql: false)
......@@ -238,6 +234,8 @@ RSpec.configure do |config|
# See https://gitlab.com/gitlab-org/gitlab/-/issues/33867
stub_feature_flags(file_identifier_hash: false)
stub_feature_flags(unified_diff_components: false)
allow(Gitlab::GitalyClient).to receive(:can_use_disk?).and_return(enable_rugged)
else
unstub_all_feature_flags
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7