Skip to content

Commit

Permalink
Selection: fixed selection on Linux with virtual folders like /dev
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
warpdesign committed Apr 23, 2024
1 parent 01d1031 commit e3b9718
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 41 deletions.
17 changes: 6 additions & 11 deletions src/services/Fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { Readable } from 'stream'

import { isWin } from '$src/utils/platform'
import { BigIntStats } from 'fs'

const interfaces: Array<Fs> = []

Expand All @@ -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
Expand Down Expand Up @@ -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<BigIntStats>, 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 {
Expand Down Expand Up @@ -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
Expand Down
31 changes: 16 additions & 15 deletions src/services/__tests__/Fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 1 addition & 4 deletions src/services/plugins/FsFtp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down
6 changes: 3 additions & 3 deletions src/services/plugins/FsLocal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -315,7 +315,7 @@ export class LocalApi implements FsApi {
isDirectory: (): boolean => isDir,
mode: -1n,
isSymbolicLink: (): boolean => isSymLink,
ino: 0n,
ino: 1n,
dev: 0n,
}
}
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/services/plugins/FsVirtual.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions src/state/fileState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('.'))) {
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit e3b9718

Please sign in to comment.