Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/i18n/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,10 @@ const en = {
'sourceControl.filter.needsSync': 'Needs Sync',
'sourceControl.filter.changes': 'Changes',
'sourceControl.filter.local': 'Local',
'sourceControl.filter.remote': 'Remote',
'sourceControl.filter.remote': 'Incoming',
'sourceControl.filter.conflict': 'Conflict',
'sourceControl.filter.readyToPush': 'Ready to Push',
'sourceControl.filter.remoteChanges': 'Remote Changes',
'sourceControl.filter.remoteChanges': 'Incoming',
'sourceControl.filter.conflicts': 'Conflicts',
'sourceControl.filter.synced': 'Synced',
'sourceControl.filter.showSynced': 'Show synced',
Expand Down
4 changes: 2 additions & 2 deletions src/i18n/locales/zh-cn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@ const zhCn: Partial<Record<TranslationKey, string>> = {
'sourceControl.filter.needsSync': '待同步',
'sourceControl.filter.changes': '更改',
'sourceControl.filter.local': '本地',
'sourceControl.filter.remote': '远程',
'sourceControl.filter.remote': '传入',
'sourceControl.filter.conflict': '冲突',
'sourceControl.filter.readyToPush': '待推送',
'sourceControl.filter.remoteChanges': '远程更改',
'sourceControl.filter.remoteChanges': '传入',
'sourceControl.filter.conflicts': '冲突',
'sourceControl.filter.synced': '已同步',
'sourceControl.filter.showSynced': '显示已同步',
Expand Down
4 changes: 2 additions & 2 deletions src/i18n/locales/zh-tw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,10 @@ const zhTw: Partial<Record<TranslationKey, string>> = {
'sourceControl.filter.needsSync': '待同步',
'sourceControl.filter.changes': '變更',
'sourceControl.filter.local': '本地',
'sourceControl.filter.remote': '遠端',
'sourceControl.filter.remote': '傳入',
'sourceControl.filter.conflict': '衝突',
'sourceControl.filter.readyToPush': '待推送',
'sourceControl.filter.remoteChanges': '遠端變更',
'sourceControl.filter.remoteChanges': '傳入',
'sourceControl.filter.conflicts': '衝突',
'sourceControl.filter.synced': '已同步',
'sourceControl.filter.showSynced': '顯示已同步',
Expand Down
9 changes: 5 additions & 4 deletions src/logic/source-control/ChangeActionPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ export function defaultSyncAction(kind: SyncChangeKind): DefaultSyncAction {

/**
* Whether a change kind has something on the remote it can pull/restore —
* `remote-only` (never existed locally) and `local-deleted` (tracked file
* removed locally, still present on remote) both do. Drives whether a row
* renders the inline Download button.
* `remote-only` (never existed locally), `remote-modified` (tracked file
* changed only on the remote), and `local-deleted` (tracked file removed
* locally, still present on remote) all do. Drives whether a row renders the
* inline Download button.
*/
export function canDownload(kind: SyncChangeKind): boolean {
return kind === 'remote-only' || kind === 'local-deleted';
return kind === 'remote-only' || kind === 'remote-modified' || kind === 'local-deleted';
}
10 changes: 3 additions & 7 deletions src/logic/source-control/FileStatusAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { toChangeId, type SyncChange, type SyncChangeKind } from './types';
const KIND_BY_STATUS: Record<SyncStatus, SyncChangeKind> = {
synced: 'synced',
modified: 'local-modified',
'remote-modified': 'remote-modified',
unsynced: 'local-only',
'remote-only': 'remote-only',
'local-deleted': 'local-deleted',
Expand All @@ -16,14 +17,9 @@ const KIND_BY_STATUS: Record<SyncStatus, SyncChangeKind> = {
* Source Control `ChangeRepository` / `SourceControlViewModel` layer added in
* Phase 1.
*
* Two known gaps versus the full `SyncChangeKind` model, both pre-existing
* limits of `FileStatus` rather than anything introduced here:
* One known gap versus the full `SyncChangeKind` model, a pre-existing limit
* of `FileStatus` rather than anything introduced here:
*
* - `FileStatus.status` never distinguishes which side changed for a
* two-sided diff (`SyncStatusService.classify` collapses both directions
* into `'modified'`), so `'modified'` maps to `'local-modified'` as a
* best-effort approximation: the row therefore offers both push and pull
* regardless of which side actually changed.
* - No `FileStatus` value ever produces `'conflict'`: conflicts are only
* detected during `SyncManager.pushFiles` (via `SyncPlanner.classify`
* against a stored base sha) and resolved interactively through
Expand Down
14 changes: 11 additions & 3 deletions src/logic/sync-status-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type { TFile } from 'obsidian';
* "Deleted locally" and the other as "Remote available" instead of conflating
* the two under a single `remote-only` state.
*/
export type SyncStatus = 'synced' | 'modified' | 'unsynced' | 'remote-only' | 'local-deleted' | 'moved';
export type SyncStatus = 'synced' | 'modified' | 'remote-modified' | 'unsynced' | 'remote-only' | 'local-deleted' | 'moved';

/** The complete status record presented by a sync-status view. */
export interface FileStatus {
Expand All @@ -35,12 +35,18 @@ export interface FileStatus {
* file was previously synced locally (sync metadata exists for the path) and
* has since been removed, so it classifies as `local-deleted` rather than
* `remote-only`.
*
* When both sides exist and differ, `localChanged`/`remoteChanged` (each
* relative to the last-synced baseline sha) let `classify` tell "only the
* remote side moved" apart from "the local side moved" or "both did" —
* without them (no baseline on record) the two-sided diff falls back to the
* direction-blind `modified`.
*/
export type SyncStatusFacts =
| { movedFrom: string }
| { localExists: true; remoteExists: false }
| { localExists: false; remoteExists: true; wasTracked?: boolean }
| { localExists: true; remoteExists: true; contentsEqual: boolean };
| { localExists: true; remoteExists: true; contentsEqual: boolean; localChanged?: boolean; remoteChanged?: boolean };

/** Resolves sync facts into the one status the UI may present for a file. */
export class SyncStatusService {
Expand All @@ -51,7 +57,9 @@ export class SyncStatusService {
if ('movedFrom' in facts) return 'moved';
if (facts.localExists && !facts.remoteExists) return 'unsynced';
if (!facts.localExists && facts.remoteExists) return facts.wasTracked ? 'local-deleted' : 'remote-only';
return facts.contentsEqual ? 'synced' : 'modified';
if (facts.contentsEqual) return 'synced';
if (facts.localChanged === false && facts.remoteChanged === true) return 'remote-modified';
return 'modified';
}

get size(): number { return this.statuses.size; }
Expand Down
51 changes: 41 additions & 10 deletions src/logic/sync/SyncStatusRefreshService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,24 @@ export class SyncStatusRefreshService {
return isSyncMetadataAtPath(pathMetadata, vaultPath) && !pathMetadata.renamedFrom;
}

/** The last-synced blob sha on record for `path`, or undefined if never tracked there. */
private baseShaFor(path: string): string | undefined {
const metadata = this.dependencies.settings().syncMetadata;
const pathMetadata = metadata ? metadata[path] : undefined;
return isSyncMetadataAtPath(pathMetadata, path) ? pathMetadata.lastSyncedSha : undefined;
}

/**
* Direction facts for a two-sided diff, relative to the last-synced
* baseline: undefined for both when there is no baseline on record (the
* two-sided diff then falls back to the direction-blind `modified`).
*/
private diffDirection(path: string, localSha: string, remoteSha: string): { localChanged?: boolean; remoteChanged?: boolean } {
const baseSha = this.baseShaFor(path);
if (baseSha === undefined) return {};
return { localChanged: localSha !== baseSha, remoteChanged: remoteSha !== baseSha };
}

async reconcileOutOfBandMoves(remoteMap: Map<string, GitTreeEntry>): Promise<void> {
const orphansBySha = this.orphanedMoveSourcesBySha(remoteMap);
if (orphansBySha.size === 0) return;
Expand Down Expand Up @@ -322,13 +340,18 @@ export class SyncStatusRefreshService {
if (!current || current.file !== file) return true;
let status: FileStatus['status'] = current.status;
if (current.status !== 'moved') {
status = current.remoteSha === undefined
? this.statuses.classify({ localExists: true, remoteExists: false })
: this.statuses.classify({
const remoteSha = current.remoteSha;
if (remoteSha === undefined) {
status = this.statuses.classify({ localExists: true, remoteExists: false });
} else {
const localSha = await gitBlobSha(localContent);
status = this.statuses.classify({
localExists: true,
remoteExists: true,
contentsEqual: await gitBlobSha(localContent) === current.remoteSha,
contentsEqual: localSha === remoteSha,
...this.diffDirection(file.path, localSha, remoteSha),
});
}
}
this.statuses.set(file.path, { ...current, status, localContent });
return true;
Expand Down Expand Up @@ -430,10 +453,13 @@ export class SyncStatusRefreshService {
const binary = isBinaryPath(path);
const symlinkMode = getEffectiveSymlinkHandling(this.dependencies.settings());
const localContent = await this.readLocalContentForSha(fileOrPath, isStringPath, binary, remoteEntry.symlink, symlinkMode);
const localSha = await gitBlobSha(localContent);
const remoteSha = remoteEntry.sha;
const status = this.statuses.classify({
localExists: true,
remoteExists: true,
contentsEqual: await gitBlobSha(localContent) === remoteEntry.sha,
contentsEqual: localSha === remoteSha,
...(remoteSha !== undefined ? this.diffDirection(path, localSha, remoteSha) : {}),
});
if (status === 'synced' && remoteEntry.sha) {
await this.dependencies.syncManager().updateMetadata(path, remoteEntry.sha);
Expand All @@ -457,13 +483,18 @@ export class SyncStatusRefreshService {
this.dependencies.getNormalizedPath(path),
this.dependencies.settings().branch,
);
const status = remote.sha
? this.statuses.classify({
let status: FileStatus['status'];
if (!remote.sha) {
status = this.statuses.classify({ localExists: true, remoteExists: false });
} else {
const equal = contentsEqual(localContent, remote.content);
status = this.statuses.classify({
localExists: true,
remoteExists: true,
contentsEqual: contentsEqual(localContent, remote.content),
})
: this.statuses.classify({ localExists: true, remoteExists: false });
contentsEqual: equal,
...(equal ? {} : this.diffDirection(path, await gitBlobSha(localContent), remote.sha)),
});
}
if (status === 'synced' && remote.sha) {
await this.dependencies.syncManager().updateMetadata(path, remote.sha);
}
Expand Down
16 changes: 9 additions & 7 deletions src/ui/source-control/ChangeItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ export interface ChangeItemCallbacks {
onOpenDiff: (item: SourceControlItem) => void;
/**
* Pulls a single change into the local vault. Invoked for rows where
* {@link canDownload} is true — `remote-only` (add it locally) and
* `local-deleted` (restore it locally) — the Download button renders
* only for those kinds, so the callback never has to re-classify.
* {@link canDownload} is true — `remote-only` (add it locally),
* `remote-modified` (overwrite the local copy), and `local-deleted`
* (restore it locally) — the Download button renders only for those
* kinds, so the callback never has to re-classify.
*/
onDownload?: (item: SourceControlItem) => void;
/** Looks up a cached diff stat for a row, if one has been computed. */
Expand Down Expand Up @@ -88,10 +89,11 @@ export function renderChangeItem(
renderDiffStat(row, callbacks.getDiffStat?.(item.id));

// A change with something to pull from remote (remote-only: add it
// locally; local-deleted: restore it locally) carries a direct Download
// action so the user can pull it without first adding it to the Sync
// Queue. The button stops propagation so clicking it doesn't also
// trigger the row's open-diff/open-remote behavior.
// locally; remote-modified: overwrite the local copy; local-deleted:
// restore it locally) carries a direct Download action so the user can
// pull it without first adding it to the Sync Queue. The button stops
// propagation so clicking it doesn't also trigger the row's
// open-diff/open-remote behavior.
if (canDownload(item.kind) && callbacks.onDownload) {
renderDownloadAction(row, item, callbacks.onDownload);
}
Expand Down
4 changes: 2 additions & 2 deletions src/ui/source-control/FilterMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type { SourceControlCounts } from '../../logic/source-control/SourceContr
* bucket in the view (see `SourceControlView`) rather than via a domain
* change. Surfaced as an opt-in overview; the default stays on Needs Sync
* so a quiet workspace stays quiet.
* - **Remote / Conflict / Synced** — the matching domain filters.
* - **Incoming / Conflict / Synced** — the matching domain filters (chip id stays `remote`; the label reads "Incoming" — a file only on the remote, or changed only on the remote, is something coming *in*).
*
* "Local" (domain `changes`) is intentionally not a chip: Needs Sync already
* covers local-side changes, and a standalone local-only view added a
Expand Down Expand Up @@ -56,7 +56,7 @@ export interface FilterMenuOptions {

/**
* Renders the Source Control filter row: five chips — All / Needs Sync /
* Remote / Conflict / Synced. On mobile a single `<select>` dropdown
* Incoming / Conflict / Synced. On mobile a single `<select>` dropdown
* replaces the chips (same chip ids, counts inline as "Label (N)").
*
* Per-filter counts come straight from the ViewModel's single-source counts
Expand Down
5 changes: 4 additions & 1 deletion tests/logic/source-control/ChangeActionPolicy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ describe('canDownload', () => {
expect(canDownload('local-deleted')).toBe(true);
});

it('allows download for remote-modified (overwrite the local copy)', () => {
expect(canDownload('remote-modified')).toBe(true);
});

it('disallows download for kinds with no separate remote-restore action', () => {
expect(canDownload('local-only')).toBe(false);
expect(canDownload('local-modified')).toBe(false);
expect(canDownload('remote-modified')).toBe(false);
expect(canDownload('moved')).toBe(false);
expect(canDownload('conflict')).toBe(false);
expect(canDownload('synced')).toBe(false);
Expand Down
2 changes: 2 additions & 0 deletions tests/logic/source-control/FileStatusAdapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ describe('toSyncChanges', () => {
const statuses: FileStatus[] = [
{ path: 'synced.md', status: 'synced' },
{ path: 'modified.md', status: 'modified' },
{ path: 'remote-modified.md', status: 'remote-modified' },
{ path: 'unsynced.md', status: 'unsynced' },
{ path: 'remote.md', status: 'remote-only' },
{ path: 'gone.md', status: 'local-deleted' },
Expand All @@ -17,6 +18,7 @@ describe('toSyncChanges', () => {
expect(toSyncChanges(statuses)).toEqual([
{ id: toChangeId('synced.md'), path: 'synced.md', previousPath: undefined, kind: 'synced' },
{ id: toChangeId('modified.md'), path: 'modified.md', previousPath: undefined, kind: 'local-modified' },
{ id: toChangeId('remote-modified.md'), path: 'remote-modified.md', previousPath: undefined, kind: 'remote-modified' },
{ id: toChangeId('unsynced.md'), path: 'unsynced.md', previousPath: undefined, kind: 'local-only' },
{ id: toChangeId('remote.md'), path: 'remote.md', previousPath: undefined, kind: 'remote-only' },
{ id: toChangeId('gone.md'), path: 'gone.md', previousPath: undefined, kind: 'local-deleted' },
Expand Down
5 changes: 4 additions & 1 deletion tests/logic/sync-status-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ describe('SyncStatusService', () => {
['a previously-tracked file removed locally', { localExists: false, remoteExists: true, wasTracked: true }, 'local-deleted'],
['a never-tracked remote file with wasTracked false', { localExists: false, remoteExists: true, wasTracked: false }, 'remote-only'],
['matching local and remote content', { localExists: true, remoteExists: true, contentsEqual: true }, 'synced'],
['different local and remote content', { localExists: true, remoteExists: true, contentsEqual: false }, 'modified'],
['different content with no baseline on record', { localExists: true, remoteExists: true, contentsEqual: false }, 'modified'],
['only the local side changed since baseline', { localExists: true, remoteExists: true, contentsEqual: false, localChanged: true, remoteChanged: false }, 'modified'],
['only the remote side changed since baseline', { localExists: true, remoteExists: true, contentsEqual: false, localChanged: false, remoteChanged: true }, 'remote-modified'],
['both sides changed since baseline', { localExists: true, remoteExists: true, contentsEqual: false, localChanged: true, remoteChanged: true }, 'modified'],
])('classifies %s as %s', (_description, facts, expected) => {
expect(service.classify(facts)).toBe(expected);
});
Expand Down
30 changes: 30 additions & 0 deletions tests/logic/sync/SyncStatusRefreshService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { TFile } from 'obsidian';
import { SyncStatusRefreshService } from '../../../src/logic/sync/SyncStatusRefreshService';
import { SyncStatusService } from '../../../src/logic/sync-status-service';
import type { SyncStatusRefreshDependencies } from '../../../src/logic/sync/SyncStatusRefreshService';
import { gitBlobSha } from '../../../src/utils/git-blob-sha';

vi.mock('obsidian');

Expand Down Expand Up @@ -347,6 +348,35 @@ describe('SyncStatusRefreshService local-change handlers', () => {

expect(statuses.has('note.md')).toBe(false);
});

it('classifies remote-modified, not modified, when the local content still matches the last-synced baseline but the remote sha has moved (regression: was silently routed to push)', async () => {
const statuses = new SyncStatusService();
const file = makeFile('note.md');
const baselineContent = 'baseline content';
const baselineSha = await gitBlobSha(baselineContent);
const service = buildService(statuses, {
app: {
vault: {
read: vi.fn().mockResolvedValue(baselineContent),
readBinary: vi.fn(),
adapter: { stat: vi.fn().mockResolvedValue(null), read: vi.fn() },
},
} as never,
settings: () => ({
syncMetadata: { 'note.md': { lastSyncedSha: baselineSha, lastSyncedAt: 1 } },
vaultFolder: '',
rootPath: '',
}) as never,
});

// Row tracked at a remote sha that has since moved away from the
// baseline, while the local content read back is unchanged.
statuses.set({ file, path: 'note.md', status: 'synced', remoteSha: 'b'.repeat(40) });

await service.handleFileModified(file);

expect(statuses.get('note.md')?.status).toBe('remote-modified');
});
});

describe('identifyExtraFiles local-deleted classification', () => {
Expand Down
7 changes: 5 additions & 2 deletions tests/ui/source-control/ChangeTree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,15 @@ describe('renderChangeTree', () => {
expect(callbacks.onDownload).toHaveBeenCalledWith(items[0]);
});

it('does not render a Download button on a remote-modified row', () => {
it('renders an inline Download button on a remote-modified row when onDownload is wired', () => {
const items = [item({ id: toChangeId('c-1'), path: 'both.md', kind: 'remote-modified' })];
callbacks.onDownload = vi.fn();
renderChangeTree(container, items, new Set(), callbacks);

expect(container.querySelector('.scv-change-download')).toBeNull();
const btn = container.querySelector('.scv-change-download') as HTMLButtonElement;
expect(btn).toBeTruthy();
btn.click();
expect(callbacks.onDownload).toHaveBeenCalledWith(items[0]);
});

it('does not render an inline status subtitle (the kind label lives on the badge tooltip)', () => {
Expand Down
Loading
Loading