Skip to content

G
gitlab-ce
Commit 8e4f548b authored by Phil Hughes's avatar Phil Hughes Committed by Kushal Pandya
Browse files
Options

Overwrite parallel_diff_lines with the newly formatted diff lines 

Doing it this way allows us to:
1. Allow all existing code to work without changing a lot
2. Migrate all the code away from this property
once this feature has been fully rolled out

https://gitlab.com/gitlab-org/gitlab/-/issues/236143
parent cb41b103
Hide whitespace changes
Inline Side-by-side
Showing with 207 additions and 111 deletions
app/assets/javascripts/diffs/components/app.vue
View file @ 8e4f548b
......@@ -203,7 +203,7 @@ export default {
}
},
diffViewType() {
if (this.needsReload() || this.needsFirstLoad()) {
if (!this.glFeatures.unifiedDiffLines && (this.needsReload() || this.needsFirstLoad())) {
this.refetchDiffData();
}
this.adjustView();
......
app/assets/javascripts/diffs/components/diff_content.vue
View file @ 8e4f548b
<script>
import { mapActions, mapGetters, mapState } from 'vuex';
import { GlLoadingIcon } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form';
import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
......@@ -32,7 +33,7 @@ export default {
userAvatarLink,
DiffFileDrafts,
},
mixins: [diffLineNoteFormMixin, draftCommentsMixin],
mixins: [diffLineNoteFormMixin, draftCommentsMixin, glFeatureFlagsMixin()],
props: {
diffFile: {
type: Object,
......@@ -114,7 +115,11 @@ export default {
<inline-diff-view
v-if="isInlineView"
:diff-file="diffFile"
:diff-lines="diffFile.highlighted_diff_lines || []"
:diff-lines="
glFeatures.unifiedDiffLines
? diffFile.parallel_diff_lines
: diffFile.highlighted_diff_lines || []
"
:help-page-path="helpPagePath"
/>
<parallel-diff-view
......
app/assets/javascripts/diffs/components/diff_expansion_cell.vue
View file @ 8e4f548b
......@@ -3,6 +3,7 @@ import { mapState, mapActions } from 'vuex';
import { GlIcon } from '@gitlab/ui';
import { deprecatedCreateFlash as createFlash } from '~/flash';
import { s__ } 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 * as utils from '../store/utils';
import tooltip from '../../vue_shared/directives/tooltip';
......@@ -28,6 +29,7 @@ export default {
components: {
GlIcon,
},
mixins: [glFeatureFlagsMixin()],
props: {
fileHash: {
type: String,
......@@ -59,7 +61,11 @@ export default {
},
computed: {
...mapState({
diffViewType: state => state.diffs.diffViewType,
diffViewType(state) {
return this.glFeatures.unifiedDiffLines
? PARALLEL_DIFF_VIEW_TYPE
: state.diffs.diffViewType;
},
diffFiles: state => state.diffs.diffFiles,
}),
canExpandUp() {
......
app/assets/javascripts/diffs/components/inline_diff_view.vue
View file @ 8e4f548b
<script>
import { mapGetters, mapState } from 'vuex';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import draftCommentsMixin from '~/diffs/mixins/draft_comments';
import InlineDraftCommentRow from '~/batch_comments/components/inline_draft_comment_row.vue';
import inlineDiffTableRow from './inline_diff_table_row.vue';
......@@ -14,7 +15,7 @@ export default {
InlineDraftCommentRow,
inlineDiffExpansionRow,
},
mixins: [draftCommentsMixin],
mixins: [draftCommentsMixin, glFeatureFlagsMixin()],
props: {
diffFile: {
type: Object,
......@@ -63,37 +64,99 @@ export default {
<col />
</colgroup>
<tbody>
<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 v-if="glFeatures.unifiedDiffLines">
<template v-for="({ left, right }, index) in diffLines">
<inline-diff-expansion-row
:key="`expand-${index}`"
:file-hash="diffFile.file_hash"
:context-lines-path="diffFile.context_lines_path"
:line="left || right"
:is-top="index === 0"
:is-bottom="index + 1 === diffLinesLength"
/>
<template v-if="left">
<inline-diff-table-row
:key="`${left.line_code || index}`"
:file-hash="diffFile.file_hash"
:file-path="diffFile.file_path"
:line="left"
:is-bottom="index + 1 === diffLinesLength"
:is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
/>
<inline-diff-comment-row
:key="`icr-${left.line_code || index}`"
:diff-file-hash="diffFile.file_hash"
:line="left"
:help-page-path="helpPagePath"
:has-draft="shouldRenderDraftRow(diffFile.file_hash, left) || false"
/>
<inline-draft-comment-row
v-if="shouldRenderDraftRow(diffFile.file_hash, left)"
:key="`draft_${index}`"
:draft="draftForLine(diffFile.file_hash, left)"
: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>
</tbody>
</table>
......
app/assets/javascripts/diffs/store/actions.js
View file @ 8e4f548b
......@@ -74,7 +74,7 @@ export const fetchDiffFiles = ({ state, commit }) => {
let returnData;
if (state.useSingleDiffStyle) {
urlParams.view = state.diffViewType;
urlParams.view = window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType;
}
commit(types.SET_LOADING, true);
......@@ -114,7 +114,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
};
if (state.useSingleDiffStyle) {
urlParams.view = state.diffViewType;
urlParams.view = window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType;
}
commit(types.SET_BATCH_LOADING, true);
......@@ -178,7 +178,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => {
const urlParams = {};
if (state.useSingleDiffStyle) {
urlParams.view = state.diffViewType;
urlParams.view = window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType;
}
commit(types.SET_LOADING, true);
......
app/assets/javascripts/diffs/store/mutations.js
View file @ 8e4f548b
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { PARALLEL_DIFF_VIEW_TYPE } from '../constants';
import {
findDiffFile,
addLineReferences,
......@@ -154,7 +155,9 @@ export default {
addContextLines({
inlineLines: diffFile.highlighted_diff_lines,
parallelLines: diffFile.parallel_diff_lines,
diffViewType: state.diffViewType,
diffViewType: window.gon?.features?.unifiedDiffLines
? PARALLEL_DIFF_VIEW_TYPE
: state.diffViewType,
contextLines: lines,
bottom,
lineNumbers,
......
app/assets/javascripts/diffs/store/utils.js
View file @ 8e4f548b
......@@ -20,6 +20,77 @@ import {
} from '../constants';
import { prepareRawDiffFile } from '../diff_file';
export const isAdded = line => ['new', 'new-nonewline'].includes(line.type);
export const isRemoved = line => ['old', 'old-nonewline'].includes(line.type);
export const isUnchanged = line => !line.type;
export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type);
/**
* Pass in the inline diff lines array which gets converted
* to the parallel diff lines.
* This allows for us to convert inline diff lines to parallel
* on the frontend without needing to send any requests
* to the API.
*
* This method has been taken from the already existing backend
* implementation at lib/gitlab/diff/parallel_diff.rb
*
* @param {Object[]} diffLines - inline diff lines
*
* @returns {Object[]} parallel lines
*/
export const parallelizeDiffLines = (diffLines = []) => {
let freeRightIndex = null;
const lines = [];
for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) {
const line = diffLines[i];
if (isRemoved(line)) {
lines.push({
[LINE_POSITION_LEFT]: line,
[LINE_POSITION_RIGHT]: null,
});
if (freeRightIndex === null) {
// Once we come upon a new line it can be put on the right of this old line
freeRightIndex = index;
}
index += 1;
} else if (isAdded(line)) {
if (freeRightIndex !== null) {
// If an old line came before this without a line on the right, this
// line can be put to the right of it.
lines[freeRightIndex].right = line;
// If there are any other old lines on the left that don't yet have
// a new counterpart on the right, update the free_right_index
const nextFreeRightIndex = freeRightIndex + 1;
freeRightIndex = nextFreeRightIndex < index ? nextFreeRightIndex : null;
} else {
lines.push({
[LINE_POSITION_LEFT]: null,
[LINE_POSITION_RIGHT]: line,
});
freeRightIndex = null;
index += 1;
}
} else if (isMeta(line) || isUnchanged(line)) {
// line in the right panel is the same as in the left one
lines.push({
[LINE_POSITION_LEFT]: line,
[LINE_POSITION_RIGHT]: line,
});
freeRightIndex = null;
index += 1;
}
}
return lines;
};
export function findDiffFile(files, match, matchKey = 'file_hash') {
return files.find(file => file[matchKey] === match);
}
......@@ -280,6 +351,13 @@ function mergeTwoFiles(target, source) {
}
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 missingParallel = !file.parallel_diff_lines;
......@@ -717,74 +795,3 @@ export const getDefaultWhitespace = (queryString, cookie) => {
if (cookie === NO_SHOW_WHITESPACE) return false;
return true;
};
export const isAdded = line => ['new', 'new-nonewline'].includes(line.type);
export const isRemoved = line => ['old', 'old-nonewline'].includes(line.type);
export const isUnchanged = line => !line.type;
export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type);
/**
* Pass in the inline diff lines array which gets converted
* to the parallel diff lines.
* This allows for us to convert inline diff lines to parallel
* on the frontend without needing to send any requests
* to the API.
*
* This method has been taken from the already existing backend
* implementation at lib/gitlab/diff/parallel_diff.rb
*
* @param {Object[]} diffLines - inline diff lines
*
* @returns {Object[]} parallel lines
*/
export const parallelizeDiffLines = (diffLines = []) => {
let freeRightIndex = null;
const lines = [];
for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) {
const line = diffLines[i];
if (isRemoved(line)) {
lines.push({
[LINE_POSITION_LEFT]: line,
[LINE_POSITION_RIGHT]: null,
});
if (freeRightIndex === null) {
// Once we come upon a new line it can be put on the right of this old line
freeRightIndex = index;
}
index += 1;
} else if (isAdded(line)) {
if (freeRightIndex !== null) {
// If an old line came before this without a line on the right, this
// line can be put to the right of it.
lines[freeRightIndex].right = line;
// If there are any other old lines on the left that don't yet have
// a new counterpart on the right, update the free_right_index
const nextFreeRightIndex = freeRightIndex + 1;
freeRightIndex = nextFreeRightIndex < index ? nextFreeRightIndex : null;
} else {
lines.push({
[LINE_POSITION_LEFT]: null,
[LINE_POSITION_RIGHT]: line,
});
freeRightIndex = null;
index += 1;
}
} else if (isMeta(line) || isUnchanged(line)) {
// line in the right panel is the same as in the left one
lines.push({
[LINE_POSITION_LEFT]: line,
[LINE_POSITION_RIGHT]: line,
});
freeRightIndex = null;
index += 1;
}
}
return lines;
};
app/controllers/projects/merge_requests_controller.rb
View file @ 8e4f548b
......@@ -40,6 +40,7 @@ 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)
end
before_action do
......
config/feature_flags/development/unified_diff_lines.yml 0 → 100644
View file @ 8e4f548b
---
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
group: group::source code
type: development
default_enabled: false
spec/spec_helper.rb
View file @ 8e4f548b
......@@ -194,6 +194,10 @@ 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)
enable_rugged = example.metadata[:enable_rugged].present?
# Disable Rugged features by default
......
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