mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-07-18 23:47:50 +00:00
b20a9931e6
This PR replaces the existing `skip-to` approach for rendering large diffs with paging over the changeset of a git diff, enabling a more efficient way to render the compare view for PR, compare, and commit views. ## Problem When a user opens a compare page for a large range of commits, Forgejo calculates the entire diff (potentially multiple times) Which consumes significant CPU time on the server and produces an enormous amount of HTML for the browser to parse and display. At some point the page becomes unusable or fails to load entirely. The previous `skip-to` approach attempted to mitigate this by allowing the user to jump to a specific file or section within the diff. However, this still required the diff process and prepare the full changeset up to that point. It did not fundamentally bound the amount of work done per request: the server could still spend excessive time calculating the Diff. As a result, `skip-to` was not a reliable solution for very large diffs. TLDR; `--skip-to` is an output-ordering/presentation option, not a performance optimization. [git docs](https://git-scm.com/docs/diff-options#Documentation/diff-options.txt---skip-tofile) ## Solution This change introduces paging over the changeset of a git diff. Rather than relying on `skip-to`, the diff is now split into manageable pages. Only a subset of the changed files is fetched, highlighted, and rendered at a time. ### Tests for Go changes - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [x] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [ ] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. <!--start release-notes-assistant--> ## Release notes <!--URL:https://codeberg.org/forgejo/forgejo--> - Security features - [PR](https://codeberg.org/forgejo/forgejo/pulls/12808): <!--number 12808 --><!--line 0 --><!--description ZG9uJ3QgcmVuZGVyIGxhcmdlIGNvbXBhcmUgZGlmZg==-->don't render large compare diff<!--description--> <!--end release-notes-assistant--> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12808 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
108 lines
3.1 KiB
Vue
108 lines
3.1 KiB
Vue
<script>
|
|
import {SvgIcon} from '../svg.js';
|
|
import {diffTreeStore} from '../modules/stores.js';
|
|
import {loadMoreFiles} from '../features/repo-diff.js';
|
|
|
|
export default {
|
|
components: {SvgIcon},
|
|
props: {
|
|
item: {
|
|
type: Object,
|
|
required: true,
|
|
},
|
|
},
|
|
data: () => ({
|
|
store: diffTreeStore(),
|
|
collapsed: false,
|
|
}),
|
|
methods: {
|
|
getIconForDiffType(pType) {
|
|
const diffTypes = {
|
|
1: {name: 'octicon-diff-added', classes: ['text', 'green']},
|
|
2: {name: 'octicon-diff-modified', classes: ['text', 'yellow']},
|
|
3: {name: 'octicon-diff-removed', classes: ['text', 'red']},
|
|
4: {name: 'octicon-diff-renamed', classes: ['text', 'teal']},
|
|
5: {name: 'octicon-diff-renamed', classes: ['text', 'green']}, // there is no octicon for copied, so renamed should be ok
|
|
};
|
|
return diffTypes[pType];
|
|
},
|
|
async loadMoreData() {
|
|
const params = new URLSearchParams(window.location.search);
|
|
params.set('file-only', true);
|
|
|
|
for (let page = this.store.currentPage + 1; page <= this.item.file.OnPage; page++) {
|
|
params.set('diff-page', page);
|
|
await loadMoreFiles(`?${params.toString()}`);
|
|
}
|
|
window.location.hash = `#diff-${this.item.file.NameHash}`;
|
|
},
|
|
},
|
|
};
|
|
</script>
|
|
<template>
|
|
<!--title instead of tooltip above as the tooltip needs too much work with the current methods, i.e. not being loaded or staying open for "too long"-->
|
|
<a
|
|
v-if="item.isFile" class="item-file"
|
|
:class="{'selected': store.selectedItem === '#diff-' + item.file.NameHash, 'viewed': item.file.IsViewed}"
|
|
:title="item.name" @click.prevent="() => loadMoreData()"
|
|
>
|
|
<!-- file -->
|
|
<SvgIcon name="octicon-file"/>
|
|
<span class="gt-ellipsis tw-flex-1">{{ item.name }}</span>
|
|
<SvgIcon :name="getIconForDiffType(item.file.Type).name" :class="getIconForDiffType(item.file.Type).classes"/>
|
|
</a>
|
|
<div v-else class="item-directory" :title="item.name" @click.stop="collapsed = !collapsed">
|
|
<!-- directory -->
|
|
<SvgIcon :name="collapsed ? 'octicon-chevron-right' : 'octicon-chevron-down'"/>
|
|
<SvgIcon class="text primary" name="octicon-file-directory-fill"/>
|
|
<span class="gt-ellipsis">{{ item.name }}</span>
|
|
</div>
|
|
|
|
<div v-if="item.children?.length" v-show="!collapsed" class="sub-items">
|
|
<DiffFileTreeItem v-for="childItem in item.children" :key="childItem.name" :item="childItem"/>
|
|
</div>
|
|
</template>
|
|
<style scoped>
|
|
a, a:hover {
|
|
text-decoration: none;
|
|
color: var(--color-text);
|
|
}
|
|
|
|
.sub-items {
|
|
display: flex;
|
|
flex-direction: column;
|
|
gap: 1px;
|
|
margin-inline-start: 13px;
|
|
border-inline-start: 1px solid var(--color-secondary);
|
|
}
|
|
|
|
.sub-items .item-file {
|
|
padding-inline-start: 18px;
|
|
}
|
|
|
|
.item-file.selected {
|
|
color: var(--color-text);
|
|
background: var(--color-active);
|
|
border-radius: 4px;
|
|
}
|
|
|
|
.item-file.viewed {
|
|
color: var(--color-text-light-3);
|
|
}
|
|
|
|
.item-file,
|
|
.item-directory {
|
|
display: flex;
|
|
align-items: center;
|
|
gap: 0.25em;
|
|
padding: 3px 6px;
|
|
}
|
|
|
|
.item-file:hover,
|
|
.item-directory:hover {
|
|
color: var(--color-text);
|
|
background: var(--color-hover);
|
|
border-radius: 4px;
|
|
cursor: pointer;
|
|
}
|
|
</style>
|