Add new UI elements for showing the (change of) file mode
Files in Git can only have 4 different modes. Add an enum for that.
The backend has recently added `new_mode` and `old_mode` properties
to the `FileInfo` entity, so let's make use of that and show this
information to the user.
For regular files nothing will change. We don't show any information
when adding, deleting or modifying regular files.
But when the file mode changes we show a warning icon. And if the new
file is not regular, then we show the file mode after the file name.
We also show a tooltip with details about the file mode.
In the diff view we expand the octal file mode to something readable.
For testing we have created a dedicated change with various file modes
and file mode changes:
https://testing-review.googlesource.com/c/file-modes/+/1501
Screenshot https://imgur.com/a/r8zC8kq
Release-Notes: skip
Google-Bug-Id: b/251087598
Change-Id: I79f4a5dbe7613900a4799930a2f6369e398d96ba
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index b52e9ae..1768552 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -619,6 +619,8 @@
lines_deleted?: number;
size_delta?: number; // in bytes
size?: number; // in bytes
+ old_mode?: number;
+ new_mode?: number;
}
/**
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 5ef8c9d..82a1319 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -60,7 +60,14 @@
import {changeModelToken} from '../../../models/change/change-model';
import {filesModelToken} from '../../../models/change/files-model';
import {ShortcutController} from '../../lit/shortcut-controller';
-import {css, html, LitElement, nothing, PropertyValues} from 'lit';
+import {
+ css,
+ html,
+ LitElement,
+ nothing,
+ PropertyValues,
+ TemplateResult,
+} from 'lit';
import {Shortcut} from '../../../services/shortcuts/shortcuts-config';
import {fire} from '../../../utils/event-util';
import {a11yStyles} from '../../../styles/gr-a11y-styles';
@@ -77,6 +84,7 @@
import {createChangeUrl} from '../../../models/views/change';
import {userModelToken} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {FileMode, fileModeToString} from '../../../utils/file-util';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -592,6 +600,16 @@
top: 2px;
display: block;
}
+ .file-mode-warning {
+ font-size: 16px;
+ position: relative;
+ top: 2px;
+ color: var(--warning-foreground);
+ }
+ .file-mode-content {
+ display: inline-block;
+ color: var(--deemphasized-text-color);
+ }
@media screen and (max-width: 1200px) {
gr-endpoint-decorator.extra-col {
@@ -1102,10 +1120,14 @@
</div>`;
}
- private renderDivWithTooltip(content: string, tooltip: string) {
+ private renderDivWithTooltip(
+ content: TemplateResult | string,
+ tooltip: string,
+ cssClass = 'content'
+ ) {
return html`
<gr-tooltip-content title=${tooltip} has-tooltip>
- <div class="content">${content}</div>
+ <div class=${cssClass}>${content}</div>
</gr-tooltip-content>
`;
}
@@ -1184,6 +1206,7 @@
>
${computeTruncatedPath(file.__path)}
</span>
+ ${this.renderFileMode(file)}
<gr-copy-clipboard
?hideInput=${true}
.text=${file.__path}
@@ -1205,6 +1228,34 @@
`;
}
+ private renderFileMode(file: NormalizedFileInfo) {
+ const {old_mode, new_mode} = file;
+
+ // For added, modified or deleted regular files we do not want to render
+ // anything. Only if a file changed from something else to regular, then let
+ // the user know.
+ if (new_mode === undefined) return nothing;
+ let newModeStr = fileModeToString(new_mode, false);
+ if (new_mode === FileMode.REGULAR_FILE) {
+ if (old_mode === undefined) return nothing;
+ if (old_mode === FileMode.REGULAR_FILE) return nothing;
+ newModeStr = `non-${fileModeToString(old_mode, false)}`;
+ }
+
+ const changed = old_mode !== undefined && old_mode !== new_mode;
+ const icon = changed
+ ? html`<gr-icon icon="warning" class="file-mode-warning"></gr-icon> `
+ : '';
+ const action = changed
+ ? `changed from ${fileModeToString(old_mode)} to`
+ : 'is';
+ return this.renderDivWithTooltip(
+ html`${icon}(${newModeStr})`,
+ `file mode ${action} ${fileModeToString(new_mode)}`,
+ 'file-mode-content'
+ );
+ }
+
private renderStyledPath(filePath: string, previousFilePath?: string) {
const {matchingFolders, newFolders, fileName} = diffFilePaths(
filePath,
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index a7d9e28..d920634 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -56,6 +56,7 @@
import {fixture, html, assert} from '@open-wc/testing';
import {Modifier} from '../../../utils/dom-util';
import {testResolver} from '../../../test/common-test-setup';
+import {FileMode} from '../../../utils/file-util';
suite('gr-diff a11y test', () => {
test('audit', async () => {
@@ -318,6 +319,44 @@
);
});
+ test('renders file mode', async () => {
+ element.files = createFiles(1, {
+ old_mode: FileMode.REGULAR_FILE,
+ new_mode: FileMode.EXECUTABLE_FILE,
+ });
+ await element.updateComplete;
+ const fileRows = queryAll<HTMLDivElement>(element, '.file-row');
+ const fileMode = queryAndAssert(
+ fileRows?.[0],
+ '.path gr-tooltip-content'
+ );
+ assert.dom.equal(
+ fileMode,
+ /* HTML */ `
+ <gr-tooltip-content
+ has-tooltip=""
+ title="file mode changed from regular (100644) to executable (100755)"
+ >
+ <div class="file-mode-content">
+ <gr-icon class="file-mode-warning" icon="warning"> </gr-icon>
+ (executable)
+ </div>
+ </gr-tooltip-content>
+ `
+ );
+ });
+
+ test('renders file mode, but not for regular files', async () => {
+ element.files = createFiles(3, {
+ old_mode: FileMode.REGULAR_FILE,
+ new_mode: FileMode.REGULAR_FILE,
+ });
+ await element.updateComplete;
+ const fileRows = queryAll<HTMLDivElement>(element, '.file-row');
+ const fileMode = query(fileRows?.[0], '.path gr-tooltip-content');
+ assert.notOk(fileMode);
+ });
+
test('renders file status column header', async () => {
element.files = createFiles(1, {lines_inserted: 9});
element.filesLeftBase = createFiles(1, {lines_inserted: 9});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 53c2780..806e7b2 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -69,6 +69,7 @@
import {grRangedCommentTheme} from '../gr-ranged-comment-themes/gr-ranged-comment-theme';
import {classMap} from 'lit/directives/class-map.js';
import {iconStyles} from '../../../styles/gr-icon-styles';
+import {expandFileMode} from '../../../utils/file-util';
const NO_NEWLINE_LEFT = 'No newline at end of left file.';
const NO_NEWLINE_RIGHT = 'No newline at end of right file.';
@@ -1765,19 +1766,18 @@
// Private but used in tests.
computeDiffHeaderItems() {
- if (!this.diff || !this.diff.diff_header) {
- return [];
- }
- return this.diff.diff_header.filter(
- item =>
- !(
- item.startsWith('diff --git ') ||
- item.startsWith('index ') ||
- item.startsWith('+++ ') ||
- item.startsWith('--- ') ||
- item === 'Binary files differ'
- )
- );
+ return (this.diff?.diff_header ?? [])
+ .filter(
+ item =>
+ !(
+ item.startsWith('diff --git ') ||
+ item.startsWith('index ') ||
+ item.startsWith('+++ ') ||
+ item.startsWith('--- ') ||
+ item === 'Binary files differ'
+ )
+ )
+ .map(expandFileMode);
}
private handleFullBypass() {
diff --git a/polygerrit-ui/app/utils/file-util.ts b/polygerrit-ui/app/utils/file-util.ts
new file mode 100644
index 0000000..246ac20
--- /dev/null
+++ b/polygerrit-ui/app/utils/file-util.ts
@@ -0,0 +1,45 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/** See also Patch.java for the backend equivalent. */
+export enum FileMode {
+ /** Mode indicating an entry is a symbolic link. */
+ SYMLINK = 0o120000,
+
+ /** Mode indicating an entry is a non-executable file. */
+ REGULAR_FILE = 0o100644,
+
+ /** Mode indicating an entry is an executable file. */
+ EXECUTABLE_FILE = 0o100755,
+
+ /** Mode indicating an entry is a submodule commit in another repository. */
+ GITLINK = 0o160000,
+}
+
+export function fileModeToString(mode?: number, includeNumber = true): string {
+ const str = fileModeStr(mode);
+ const num = mode?.toString(8);
+ return `${str}${includeNumber && str ? ` (${num})` : ''}`;
+}
+
+function fileModeStr(mode?: number): string {
+ if (mode === FileMode.SYMLINK) return 'symlink';
+ if (mode === FileMode.REGULAR_FILE) return 'regular';
+ if (mode === FileMode.EXECUTABLE_FILE) return 'executable';
+ if (mode === FileMode.GITLINK) return 'gitlink';
+ return '';
+}
+
+export function expandFileMode(input?: string) {
+ if (!input) return input;
+ for (const modeNum of Object.values(FileMode) as FileMode[]) {
+ const modeStr = modeNum?.toString(8);
+ if (input.includes(modeStr)) {
+ return input.replace(modeStr, `${fileModeToString(modeNum)}`);
+ }
+ }
+ return input;
+}
diff --git a/polygerrit-ui/app/utils/file-util_test.ts b/polygerrit-ui/app/utils/file-util_test.ts
new file mode 100644
index 0000000..aeab026
--- /dev/null
+++ b/polygerrit-ui/app/utils/file-util_test.ts
@@ -0,0 +1,38 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {assert} from '@open-wc/testing';
+import '../test/common-test-setup';
+import {expandFileMode, FileMode, fileModeToString} from './file-util';
+
+suite('file-util tests', () => {
+ test('fileModeToString', () => {
+ const check = (
+ mode: number | undefined,
+ str: string,
+ includeNumber = true
+ ) => assert.equal(fileModeToString(mode, includeNumber), str);
+
+ check(undefined, '');
+ check(0, '');
+ check(1, '');
+ check(FileMode.REGULAR_FILE, 'regular', false);
+ check(FileMode.EXECUTABLE_FILE, 'executable', false);
+ check(FileMode.SYMLINK, 'symlink', false);
+ check(FileMode.GITLINK, 'gitlink', false);
+ check(FileMode.REGULAR_FILE, 'regular (100644)');
+ check(FileMode.EXECUTABLE_FILE, 'executable (100755)');
+ check(FileMode.SYMLINK, 'symlink (120000)');
+ check(FileMode.GITLINK, 'gitlink (160000)');
+ });
+
+ test('expandFileMode', () => {
+ assert.deepEqual(['asdf'].map(expandFileMode), ['asdf']);
+ assert.deepEqual(
+ ['old mode 100644', 'new mode 100755'].map(expandFileMode),
+ ['old mode regular (100644)', 'new mode executable (100755)']
+ );
+ });
+});
