Skip to content

G
gitlab-ce
Commit 87b0c6c9 authored by Igor Drozdov's avatar Igor Drozdov
Browse files
Options

Merge branch 'ph/241188/removeUnifiedDiffLinesFeatureFlag' into 'master' 

Remove the unified diff lines feature flag

See merge request gitlab-org/gitlab!46024
parents eb4dd716 aa84c42f
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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ eb4dd716
---
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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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 @ 87b0c6c9
This diff is collapsed.
spec/frontend/diffs/store/utils_spec.js
View file @ 87b0c6c9
This diff is collapsed.
spec/helpers/diff_helper_spec.rb
View file @ 87b0c6c9
......@@ -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 @ 87b0c6c9
......@@ -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