diff --git a/src-tauri/src/git/remote.rs b/src-tauri/src/git/remote.rs index 507067b6..86dad2e6 100644 --- a/src-tauri/src/git/remote.rs +++ b/src-tauri/src/git/remote.rs @@ -4,6 +4,13 @@ use std::path::Path; use super::conflict::get_conflict_files; +const GIT_CONFIG_SUBCOMMAND: &str = "config"; +const GIT_CONFIG_GET_REGEXP: &str = "--get-regexp"; +const REMOTE_URL_CONFIG_PATTERN: &str = r"^remote\..*\.url$"; +const NO_REMOTE_STATUS: &str = "no_remote"; +const NO_REMOTE_MESSAGE: &str = "No remote configured"; +const GIT_INSPECT_REMOTES_ERROR: &str = "Failed to inspect git remotes"; + #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] pub struct GitPullResult { pub status: String, // "up_to_date" | "updated" | "conflict" | "no_remote" | "error" @@ -17,8 +24,16 @@ pub struct GitPullResult { /// Check whether the vault repo has at least one remote configured. pub fn has_remote(vault_path: &str) -> Result { let vault = Path::new(vault_path); + remote_url_config_exists(vault) +} + +fn remote_url_config_exists(vault: &Path) -> Result { let output = git_command() - .args(["config", "--get-regexp", r"^remote\..*\.url$"]) + .args([ + GIT_CONFIG_SUBCOMMAND, + GIT_CONFIG_GET_REGEXP, + REMOTE_URL_CONFIG_PATTERN, + ]) .current_dir(vault) .output() .map_err(|e| format!("Failed to inspect git remotes: {}", e))?; @@ -29,7 +44,7 @@ pub fn has_remote(vault_path: &str) -> Result { } let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string(); return Err(if stderr.is_empty() { - "Failed to inspect git remotes".to_string() + GIT_INSPECT_REMOTES_ERROR.to_string() } else { stderr }); @@ -45,8 +60,8 @@ pub fn git_pull(vault_path: &str) -> Result { if !has_remote(vault_path)? { return Ok(GitPullResult { - status: "no_remote".to_string(), - message: "No remote configured".to_string(), + status: NO_REMOTE_STATUS.to_string(), + message: NO_REMOTE_MESSAGE.to_string(), updated_files: vec![], conflict_files: vec![], }); @@ -313,8 +328,8 @@ pub fn git_push(vault_path: &str) -> Result { if !has_remote(vault_path)? { return Ok(GitPushResult { - status: "no_remote".to_string(), - message: "No remote configured".to_string(), + status: NO_REMOTE_STATUS.to_string(), + message: NO_REMOTE_MESSAGE.to_string(), }); } @@ -342,6 +357,11 @@ mod tests { use crate::git::tests::{setup_git_repo, setup_remote_pair}; use std::fs; + fn commit_default_note(vault_path: &Path) { + fs::write(vault_path.join("note.md"), "# Note\n").unwrap(); + git_commit(vault_path.to_str().unwrap(), "initial").unwrap(); + } + #[test] fn test_has_remote_returns_false_for_local_repo() { let dir = setup_git_repo(); @@ -527,8 +547,7 @@ hint: have locally."#; let (_bare, clone_a, _clone_b) = setup_remote_pair(); let vp_a = clone_a.path().to_str().unwrap(); - fs::write(clone_a.path().join("note.md"), "# Note\n").unwrap(); - git_commit(vp_a, "initial").unwrap(); + commit_default_note(clone_a.path()); let result = git_push(vp_a).unwrap(); assert_eq!(result.status, "ok"); } @@ -539,8 +558,7 @@ hint: have locally."#; let vault = dir.path(); let vp = vault.to_str().unwrap(); - fs::write(vault.join("note.md"), "# Note\n").unwrap(); - git_commit(vp, "initial").unwrap(); + commit_default_note(vault); let result = git_push(vp).unwrap(); assert_eq!(result.status, "no_remote"); diff --git a/src/components/FolderTree.test.tsx b/src/components/FolderTree.test.tsx index fbaf873a..b50fbd04 100644 --- a/src/components/FolderTree.test.tsx +++ b/src/components/FolderTree.test.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { useState, type ComponentProps } from 'react' import { act, fireEvent, render, screen, within } from '@testing-library/react' import { describe, it, expect, vi } from 'vitest' import { FolderTree } from './FolderTree' @@ -22,6 +22,30 @@ const mockFolders: FolderNode[] = [ const defaultSelection: SidebarSelection = { kind: 'filter', filter: 'all' } const vaultRootPath = '/Users/luca/Laputa' +function renderTree(props: Partial> = {}) { + const onSelect = props.onSelect ?? vi.fn() + render( + , + ) + return { onSelect } +} + +function clickFolderRow(path: string) { + fireEvent.click(screen.getByTestId(`folder-row:${path}`)) +} + +async function submitNewFolder(name: string) { + fireEvent.click(screen.getByTestId('create-folder-btn')) + const input = screen.getByTestId('new-folder-input') + fireEvent.change(input, { target: { value: name } }) + fireEvent.keyDown(input, { key: 'Enter' }) +} + describe('FolderTree', () => { it('renders nothing when folders is empty', () => { const { container } = render( @@ -153,32 +177,16 @@ describe('FolderTree', () => { }) it('selects the vault root with the root path attached', () => { - const onSelect = vi.fn() - render( - , - ) + const { onSelect } = renderTree({ vaultRootPath }) - fireEvent.click(screen.getByTestId('folder-row:')) + clickFolderRow('') expect(onSelect).toHaveBeenCalledWith({ kind: 'folder', path: '', rootPath: vaultRootPath }) }) it('selects child folders with the vault root path attached when the tree has a root', () => { - const onSelect = vi.fn() - render( - , - ) + const { onSelect } = renderTree({ vaultRootPath }) - fireEvent.click(screen.getByTestId('folder-row:areas')) + clickFolderRow('areas') expect(onSelect).toHaveBeenCalledWith({ kind: 'folder', path: 'areas', rootPath: vaultRootPath }) }) @@ -237,20 +245,13 @@ describe('FolderTree', () => { it('passes the selected folder as the parent when creating a new folder', async () => { const onCreateFolder = vi.fn().mockResolvedValue(true) - render( - , - ) + renderTree({ + selection: { kind: 'folder', path: 'projects', rootPath: vaultRootPath }, + onCreateFolder, + vaultRootPath, + }) - fireEvent.click(screen.getByTestId('create-folder-btn')) - const input = screen.getByTestId('new-folder-input') - fireEvent.change(input, { target: { value: 'laputa' } }) - fireEvent.keyDown(input, { key: 'Enter' }) + await submitNewFolder('laputa') await vi.waitFor(() => { expect(onCreateFolder).toHaveBeenCalledWith('laputa', { @@ -278,20 +279,14 @@ describe('FolderTree', () => { }, ] - render( - , - ) + renderTree({ + folders, + selection: { kind: 'folder', path: '', rootPath: otherVault }, + onCreateFolder, + vaultRootPath, + }) - fireEvent.click(screen.getByTestId('create-folder-btn')) - const input = screen.getByTestId('new-folder-input') - fireEvent.change(input, { target: { value: 'inbox' } }) - fireEvent.keyDown(input, { key: 'Enter' }) + await submitNewFolder('inbox') await vi.waitFor(() => { expect(onCreateFolder).toHaveBeenCalledWith('inbox', { @@ -303,20 +298,9 @@ describe('FolderTree', () => { it('omits parent context when nothing folder-like is selected', async () => { const onCreateFolder = vi.fn().mockResolvedValue(true) - render( - , - ) + renderTree({ onCreateFolder, vaultRootPath }) - fireEvent.click(screen.getByTestId('create-folder-btn')) - const input = screen.getByTestId('new-folder-input') - fireEvent.change(input, { target: { value: 'inbox' } }) - fireEvent.keyDown(input, { key: 'Enter' }) + await submitNewFolder('inbox') await vi.waitFor(() => { expect(onCreateFolder).toHaveBeenCalledWith('inbox', undefined) diff --git a/src/components/FolderTree.tsx b/src/components/FolderTree.tsx index b35b89c9..dabcffd1 100644 --- a/src/components/FolderTree.tsx +++ b/src/components/FolderTree.tsx @@ -94,6 +94,39 @@ function useDisplayedFolders(folders: FolderNode[], expanded: Record void + expandFolder: (key: string) => void + onCreateFolder?: (name: string, parent?: FolderCreationParent) => Promise | boolean + selection: SidebarSelection +}) { + return useCallback(async (value: string) => { + const nextName = value.trim() + if (!nextName || !onCreateFolder) { + closeCreateForm() + return true + } + + const parent = creationParentForSelection(selection) + const created = await onCreateFolder(nextName, parent) + if (!created) return created + + closeCreateForm() + if (parent?.path) expandFolder(folderNodeKey(parent)) + return created + }, [closeCreateForm, expandFolder, onCreateFolder, selection]) +} + export const FolderTree = memo(function FolderTree({ folders, selection, @@ -140,29 +173,12 @@ export const FolderTree = memo(function FolderTree({ onStartRenameFolder, }) - const handleCreateFolderSubmit = useCallback(async (value: string) => { - const nextName = value.trim() - if (!nextName || !onCreateFolder) { - closeCreateForm() - return true - } - - const parent: FolderCreationParent | undefined = selection.kind === 'folder' - ? { path: selection.path, rootPath: selection.rootPath } - : undefined - const created = await onCreateFolder(nextName, parent) - if (created) { - closeCreateForm() - // Ensure the parent folder is open so the freshly created child is visible. - // The selection→ancestor-expansion machinery deliberately doesn't expand the - // selected node itself, so we expand it explicitly here. Vault-root keys are - // open by default and don't need this. - if (parent?.path) { - expandFolder(folderNodeKey(parent)) - } - } - return created - }, [closeCreateForm, expandFolder, onCreateFolder, selection]) + const handleCreateFolderSubmit = useCreateFolderSubmit({ + closeCreateForm, + expandFolder, + onCreateFolder, + selection, + }) const handleCreateFolderClick = useCallback(() => { closeContextMenu() diff --git a/src/components/status-bar/VaultMenu.applyMountedChange.test.ts b/src/components/status-bar/VaultMenu.applyMountedChange.test.ts index b83aacdb..e93a8e7d 100644 --- a/src/components/status-bar/VaultMenu.applyMountedChange.test.ts +++ b/src/components/status-bar/VaultMenu.applyMountedChange.test.ts @@ -30,53 +30,48 @@ function callApply(overrides: ChangeOverrides) { } describe('applyMountedChange', () => { - it('reroutes both the default workspace and the active vault when the unmounted vault is both', () => { - const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ - defaultPath: '/a', - vaultPath: '/a', - path: '/a', - }) + it.each([ + { + name: 'both default and active vault', + request: { defaultPath: '/a', vaultPath: '/a', path: '/a' }, + expectedDefault: '/b', + expectedSwitch: '/b', + expectedIdentity: ['/a', { mounted: false }], + }, + { + name: 'active vault only', + request: { defaultPath: '/b', vaultPath: '/a', path: '/a' }, + expectedDefault: null, + expectedSwitch: '/b', + expectedIdentity: ['/a', { mounted: false }], + }, + { + name: 'default workspace only', + request: { defaultPath: '/a', vaultPath: '/b', path: '/a' }, + expectedDefault: '/b', + expectedSwitch: null, + expectedIdentity: ['/a', { mounted: false }], + }, + { + name: 'neither default nor active vault', + request: { + defaultPath: '/a', + vaultPath: '/a', + path: '/b', + includedVaults: [vault('/a'), vault('/b'), vault('/c')], + }, + expectedDefault: null, + expectedSwitch: null, + expectedIdentity: ['/b', { mounted: false }], + }, + ])('handles unmounting $name', ({ request, expectedDefault, expectedSwitch, expectedIdentity }) => { + const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply(request) - expect(onSetDefaultWorkspace).toHaveBeenCalledWith('/b') - expect(onSwitchVault).toHaveBeenCalledWith('/b') - expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false }) - }) - - it('reroutes the active vault when only the active vault is unmounted (default workspace differs)', () => { - const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ - defaultPath: '/b', - vaultPath: '/a', - path: '/a', - }) - - expect(onSetDefaultWorkspace).not.toHaveBeenCalled() - expect(onSwitchVault).toHaveBeenCalledWith('/b') - expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false }) - }) - - it('reroutes the default workspace when only the default is unmounted (active vault differs)', () => { - const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ - defaultPath: '/a', - vaultPath: '/b', - path: '/a', - }) - - expect(onSetDefaultWorkspace).toHaveBeenCalledWith('/b') - expect(onSwitchVault).not.toHaveBeenCalled() - expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/a', { mounted: false }) - }) - - it('does not reroute when the unmounted vault is neither default nor active', () => { - const { onSetDefaultWorkspace, onSwitchVault, onUpdateWorkspaceIdentity } = callApply({ - defaultPath: '/a', - vaultPath: '/a', - path: '/b', - includedVaults: [vault('/a'), vault('/b'), vault('/c')], - }) - - expect(onSetDefaultWorkspace).not.toHaveBeenCalled() - expect(onSwitchVault).not.toHaveBeenCalled() - expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith('/b', { mounted: false }) + if (expectedDefault) expect(onSetDefaultWorkspace).toHaveBeenCalledWith(expectedDefault) + else expect(onSetDefaultWorkspace).not.toHaveBeenCalled() + if (expectedSwitch) expect(onSwitchVault).toHaveBeenCalledWith(expectedSwitch) + else expect(onSwitchVault).not.toHaveBeenCalled() + expect(onUpdateWorkspaceIdentity).toHaveBeenCalledWith(...expectedIdentity) }) it('bails out without changing anything when no alternative mounted vault exists', () => { diff --git a/src/components/status-bar/vaultMenuMountedChange.ts b/src/components/status-bar/vaultMenuMountedChange.ts index a70fe92f..77023e18 100644 --- a/src/components/status-bar/vaultMenuMountedChange.ts +++ b/src/components/status-bar/vaultMenuMountedChange.ts @@ -19,6 +19,20 @@ function nextIncludedVaultPath(includedVaults: VaultOption[], currentPath: strin return includedVaults.find((vault) => vault.path !== currentPath)?.path ?? null } +function isCurrentPathUnmount(request: VaultMountChangeRequest): boolean { + if (request.mounted) return false + if (request.path === request.defaultPath) return true + return request.path === request.vaultPath +} + +function rerouteUnmountedPath(request: VaultMountChangeRequest): boolean { + const nextPath = nextIncludedVaultPath(request.includedVaults, request.path) + if (!nextPath) return false + if (request.path === request.defaultPath) request.callbacks.onSetDefaultWorkspace?.(nextPath) + if (request.path === request.vaultPath) request.callbacks.onSwitchVault(nextPath) + return true +} + export function applyMountedChange({ defaultPath, vaultPath, @@ -27,11 +41,7 @@ export function applyMountedChange({ path, callbacks, }: VaultMountChangeRequest): void { - if (!mounted && (path === defaultPath || path === vaultPath)) { - const nextPath = nextIncludedVaultPath(includedVaults, path) - if (!nextPath) return - if (path === defaultPath) callbacks.onSetDefaultWorkspace?.(nextPath) - if (path === vaultPath) callbacks.onSwitchVault(nextPath) - } + const request = { defaultPath, vaultPath, includedVaults, mounted, path, callbacks } + if (isCurrentPathUnmount(request) && !rerouteUnmountedPath(request)) return callbacks.onUpdateWorkspaceIdentity?.(path, { mounted }) } diff --git a/src/lib/aiTargets.ts b/src/lib/aiTargets.ts index 8b52e10f..b9d78182 100644 --- a/src/lib/aiTargets.ts +++ b/src/lib/aiTargets.ts @@ -124,24 +124,34 @@ export function agentTargets(): AiTarget[] { export function resolveAiTarget(settings: Settings): AiTarget { const providers = normalizeAiModelProviders(settings.ai_model_providers) - const targets = [...agentTargets(), ...configuredModelTargets(providers)] + const agents = agentTargets() + const targets = [...agents, ...configuredModelTargets(providers)] const storedLegacyAgent = normalizeStoredAiAgent(settings.default_ai_agent) const legacyAgent = storedLegacyAgent ?? DEFAULT_AI_AGENT const target = resolveStoredAiTarget(settings.default_ai_target, targets) if (target) { - if (target.kind === 'agent' && storedLegacyAgent && target.agent === DEFAULT_AI_AGENT && target.agent !== storedLegacyAgent) { - return agentTargets().find((candidate) => candidate.kind === 'agent' && candidate.agent === legacyAgent) ?? target - } + if (shouldPreferLegacyAgent(target, storedLegacyAgent)) return agentTargetFor(agents, legacyAgent) ?? target return target } - return agentTargets().find((candidate) => candidate.kind === 'agent' && candidate.agent === legacyAgent) ?? agentTargets()[0] + return agentTargetFor(agents, legacyAgent) ?? agents[0] } export function targetAgent(target: AiTarget): AiAgentId { return target.kind === 'agent' ? target.agent : DEFAULT_AI_AGENT } +function shouldPreferLegacyAgent(target: AiTarget, storedLegacyAgent: AiAgentId | null): boolean { + if (target.kind !== 'agent') return false + if (!storedLegacyAgent) return false + if (target.agent !== DEFAULT_AI_AGENT) return false + return target.agent !== storedLegacyAgent +} + +function agentTargetFor(agents: AiTarget[], agent: AiAgentId): AiTarget | undefined { + return agents.find((candidate) => candidate.kind === 'agent' && candidate.agent === agent) +} + function resolveStoredAiTarget(storedTarget: string | null | undefined, targets: AiTarget[]): AiTarget | null { const target = storedTarget?.trim() if (!target) return null