From 6588bd00a860934fa241eb949cab8825fbff30b3 Mon Sep 17 00:00:00 2001 From: Nicolas Ramz Date: Tue, 23 Apr 2024 17:39:01 +0200 Subject: [PATCH] Selection: fixed selection on Linux with virtual folders like /dev We were incorrectly using {ino,dev} as file ID but on Linux there may be virtual folders that have no unique ino like /proc, /dev. This broke selection/navigation on `/` on Linux / WSL since several folders could end up having the same ID. This PRs still uses ino + dev when possible and falls back to the string `${ino}-${dev}-${path}` for virtual folders. This fixes the navigation on `/` on *nix while still keeping nice features, like renaming a selected file will still reselect the correct file when the folder is refreshed. --- src/services/Fs.ts | 17 ++++++----------- src/services/__tests__/Fs.test.ts | 31 ++++++++++++++++--------------- src/services/plugins/FsFtp.ts | 5 +---- src/services/plugins/FsLocal.ts | 6 +++--- src/services/plugins/FsVirtual.ts | 4 ++-- src/state/fileState.ts | 8 ++------ 6 files changed, 30 insertions(+), 41 deletions(-) diff --git a/src/services/Fs.ts b/src/services/Fs.ts index 759023c9..de889570 100644 --- a/src/services/Fs.ts +++ b/src/services/Fs.ts @@ -2,6 +2,7 @@ import { Readable } from 'stream' import { isWin } from '$src/utils/platform' +import { BigIntStats } from 'fs' const interfaces: Array = [] @@ -15,10 +16,7 @@ export function registerFs(fs: Fs): void { interfaces.push(fs) } -export interface FileID { - ino: bigint - dev: bigint -} +export type FileID = string export interface FileDescriptor { dir: string @@ -71,11 +69,8 @@ export const ExeMaskUser = 0o0100 export type FileType = 'exe' | 'img' | 'arc' | 'snd' | 'vid' | 'doc' | 'cod' | '' -export function MakeId(stats: { ino: bigint; dev: bigint }): FileID { - return { - ino: stats.ino, - dev: stats.dev, - } +export function MakeId(stats: Partial, fullpath: string): FileID { + return stats.ino > 1n ? `${stats.ino}-${stats.dev}` : `${stats.ino}-${stats.dev}-${fullpath}` } function isModeExe(mode: number, gid: number, uid: number): boolean { @@ -194,8 +189,8 @@ export function needsConnection(target: any, key: any, descriptor: any) { return descriptor } -export function sameID({ ino, dev }: FileID, { ino: ino2, dev: dev2 }: FileID): boolean { - return ino === ino2 && dev === dev2 +export function sameID(id1: FileID, id2: FileID): boolean { + return id1 === id2 } // in test environment, load the generic fs as first one diff --git a/src/services/__tests__/Fs.test.ts b/src/services/__tests__/Fs.test.ts index 24ab47a2..5c5f8b3f 100644 --- a/src/services/__tests__/Fs.test.ts +++ b/src/services/__tests__/Fs.test.ts @@ -6,30 +6,31 @@ import { describeUnix } from '../../utils/test/helpers' import { MakeId, ExeMaskAll, ExeMaskGroup, ExeMaskUser, filetype, sameID, FileID } from '../Fs' describe('makeId', () => { - it('should return FileID from stats', () => { + it('should return FileID from stats using ino > 1', () => { + const fullPath = '/dev/foo' const stats = { ino: 123n, dev: 456n, - fullname: 'foo', } - expect(MakeId(stats)).toEqual({ - ino: stats.ino, - dev: stats.dev, - }) + expect(MakeId(stats, fullPath)).toEqual(`${stats.ino}-${stats.dev}`) + }) + + it('should return FileID from stats using ino <= 1', () => { + const fullPath = '/dev/foo' + const stats = { + ino: 1n, + dev: 456n, + } + + expect(MakeId(stats, fullPath)).toEqual(`${stats.ino}-${stats.dev}-${fullPath}`) }) }) describe('sameID', () => { - const id1: FileID = { - dev: 10n, - ino: 5n, - } - - const id2: FileID = { - dev: 28n, - ino: 32n, - } + const id1 = '123-456-foo' + + const id2 = '123-789-bar' it('should return true if ino & dev are identical', () => { expect(sameID(id1, id1)).toBe(true) diff --git a/src/services/plugins/FsFtp.ts b/src/services/plugins/FsFtp.ts index dc531c15..dae2a5ef 100644 --- a/src/services/plugins/FsFtp.ts +++ b/src/services/plugins/FsFtp.ts @@ -230,10 +230,7 @@ class Client { type: (ftpFile.type !== 'd' && filetype(0, 0, 0, ext)) || '', isSym: false, target: null, - id: { - ino: BigInt(mDate.getTime()), - dev: BigInt(new Date().getTime()), - }, + id: ftpFile.name, } return file }) diff --git a/src/services/plugins/FsLocal.ts b/src/services/plugins/FsLocal.ts index 8b992c58..64aca29f 100644 --- a/src/services/plugins/FsLocal.ts +++ b/src/services/plugins/FsLocal.ts @@ -227,7 +227,7 @@ export class LocalApi implements FsApi { '', isSym: stats.isSymbolicLink(), target: (stats.isSymbolicLink() && fs.readlinkSync(fullPath)) || null, - id: MakeId({ ino: stats.ino, dev: stats.dev }), + id: MakeId(stats, fullPath), } return file @@ -315,7 +315,7 @@ export class LocalApi implements FsApi { isDirectory: (): boolean => isDir, mode: -1n, isSymbolicLink: (): boolean => isSymLink, - ino: 0n, + ino: 1n, dev: 0n, } } @@ -341,7 +341,7 @@ export class LocalApi implements FsApi { '', isSym: stats.isSymbolicLink(), target: (stats.isSymbolicLink() && name) || null, - id: MakeId({ ino: stats.ino, dev: stats.dev }), + id: MakeId(stats, fullPath), } return file diff --git a/src/services/plugins/FsVirtual.ts b/src/services/plugins/FsVirtual.ts index 420cb307..dc5c742d 100644 --- a/src/services/plugins/FsVirtual.ts +++ b/src/services/plugins/FsVirtual.ts @@ -228,7 +228,7 @@ export class VirtualApi implements FsApi { '', isSym: stats.isSymbolicLink(), target: (stats.isSymbolicLink() && vol.readlinkSync(fullPath)) || null, - id: MakeId({ ino: stats.ino, dev: stats.dev }), + id: MakeId({ ino: stats.ino, dev: stats.dev }, fullPath), } return file @@ -340,7 +340,7 @@ export class VirtualApi implements FsApi { '', isSym: stats.isSymbolicLink(), target: (stats.isSymbolicLink() && name) || null, - id: MakeId({ ino: stats.ino, dev: stats.dev }), + id: MakeId({ ino: stats.ino, dev: stats.dev }, fullPath), } return file diff --git a/src/state/fileState.ts b/src/state/fileState.ts index 3aafdf41..7e781519 100644 --- a/src/state/fileState.ts +++ b/src/state/fileState.ts @@ -375,9 +375,7 @@ export class FileState { for (const selection of this.selected) { // use inode/dev to retrieve files that were selected before reload: // we cannot use fullname anymore since files may have been renamed - const newFile = this.files.find( - (file) => file.id.dev === selection.id.dev && file.id.ino === selection.id.ino, - ) + const newFile = this.files.find((file) => file.id === selection.id) // don't add file to selection list if it is supposed to be hidden and we don't // want to show hidden files if (newFile && (this.showHiddenFiles || !newFile.fullname.startsWith('.'))) { @@ -499,9 +497,7 @@ export class FileState { setEditingFile(file: FileDescriptor): void { if (file) { - this.editingId = { - ...file.id, - } + this.editingId = file.id } else { this.editingId = null }