Skip to content
Open
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
27 changes: 27 additions & 0 deletions apps/desktop/src/lib/trpc/routers/settings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
DEFAULT_AUTO_APPLY_DEFAULT_PRESET,
DEFAULT_CONFIRM_ON_QUIT,
DEFAULT_FILE_OPEN_MODE,
DEFAULT_NEW_TERMINAL_IN_CURRENT_TAB_DIRECTORY,
DEFAULT_OPEN_LINKS_IN_APP,
DEFAULT_SHOW_PRESETS_BAR,
DEFAULT_SHOW_RESOURCE_MONITOR,
Expand Down Expand Up @@ -436,6 +437,32 @@ export const createSettingsRouter = () => {
return { success: true };
}),

getNewTerminalInCurrentTabDirectory: publicProcedure.query(() => {
const row = getSettings();
return (
row.newTerminalInCurrentTabDirectory ??
DEFAULT_NEW_TERMINAL_IN_CURRENT_TAB_DIRECTORY
);
}),

setNewTerminalInCurrentTabDirectory: publicProcedure
.input(z.object({ enabled: z.boolean() }))
.mutation(({ input }) => {
localDb
.insert(settings)
.values({
id: 1,
newTerminalInCurrentTabDirectory: input.enabled,
})
.onConflictDoUpdate({
target: settings.id,
set: { newTerminalInCurrentTabDirectory: input.enabled },
})
.run();

return { success: true };
}),

getTerminalLinkBehavior: publicProcedure.query(() => {
const row = getSettings();
return row.terminalLinkBehavior ?? DEFAULT_TERMINAL_LINK_BEHAVIOR;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
type SettingItemId,
} from "../../../utils/settings-search";
import { LinkBehaviorSetting } from "./components/LinkBehaviorSetting";
import { NewTerminalDirectorySetting } from "./components/NewTerminalDirectorySetting";
import { PresetsSection } from "./components/PresetsSection";
import { SessionsSection } from "./components/SessionsSection";

Expand Down Expand Up @@ -52,6 +53,10 @@ export function TerminalSettings({
SETTING_ITEM_ID.TERMINAL_LINK_BEHAVIOR,
visibleItems,
);
const showNewTerminalDirectory = isItemVisible(
SETTING_ITEM_ID.TERMINAL_NEW_TERMINAL_DIRECTORY,
visibleItems,
);
const showSessions = isItemVisible(
SETTING_ITEM_ID.TERMINAL_SESSIONS,
visibleItems,
Expand All @@ -76,6 +81,9 @@ export function TerminalSettings({
onEditingPresetIdChange={onEditingPresetIdChange}
/>
)}
{showNewTerminalDirectory && (
<NewTerminalDirectorySetting key="new-terminal-directory" />
)}
{showLinkBehavior && <LinkBehaviorSetting key="link-behavior" />}
{showSessions && <SessionsSection key="sessions" />}
</SectionList>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { Label } from "@superset/ui/label";
import { Switch } from "@superset/ui/switch";
import { electronTrpc } from "renderer/lib/electron-trpc";

export function NewTerminalDirectorySetting() {
const utils = electronTrpc.useUtils();
const { data: enabled, isLoading } =
electronTrpc.settings.getNewTerminalInCurrentTabDirectory.useQuery();
const setEnabled =
electronTrpc.settings.setNewTerminalInCurrentTabDirectory.useMutation({
onMutate: async ({ enabled: nextEnabled }) => {
await utils.settings.getNewTerminalInCurrentTabDirectory.cancel();
const previous =
utils.settings.getNewTerminalInCurrentTabDirectory.getData();
utils.settings.getNewTerminalInCurrentTabDirectory.setData(
undefined,
nextEnabled,
);
return { previous };
},
onError: (_err, _vars, context) => {
if (context?.previous !== undefined) {
utils.settings.getNewTerminalInCurrentTabDirectory.setData(
undefined,
context.previous,
);
}
},
onSettled: () => {
utils.settings.getNewTerminalInCurrentTabDirectory.invalidate();
},
});

return (
<div className="flex items-center justify-between">
<div className="space-y-0.5">
<Label
htmlFor="new-terminal-in-current-tab-directory"
className="text-sm font-medium"
>
New terminals follow current tab directory
</Label>
<p className="text-xs text-muted-foreground">
Open new terminals in the active tab&apos;s directory when Superset
can resolve one
</p>
</div>
<Switch
id="new-terminal-in-current-tab-directory"
checked={enabled ?? true}
onCheckedChange={(nextEnabled) =>
setEnabled.mutate({ enabled: nextEnabled })
}
disabled={isLoading || setEnabled.isPending}
/>
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,10 @@ describe("settings search - font settings", () => {
expect(editorFont?.section).toBe("appearance");
expect(terminalFont?.section).toBe("appearance");
});

it('searching "directory" returns the new terminal directory setting', () => {
const results = searchSettings("directory");
const ids = getIds(results);
expect(ids).toContain(SETTING_ITEM_ID.TERMINAL_NEW_TERMINAL_DIRECTORY);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export const SETTING_ITEM_ID = {

TERMINAL_PRESETS: "terminal-presets",
TERMINAL_QUICK_ADD: "terminal-quick-add",
TERMINAL_NEW_TERMINAL_DIRECTORY: "terminal-new-terminal-directory",
TERMINAL_SESSIONS: "terminal-sessions",
TERMINAL_LINK_BEHAVIOR: "terminal-link-behavior",

Expand Down Expand Up @@ -518,6 +519,24 @@ export const SETTINGS_ITEMS: SettingsItem[] = [
"assistant",
],
},
{
id: SETTING_ITEM_ID.TERMINAL_NEW_TERMINAL_DIRECTORY,
section: "terminal",
title: "New terminal directory",
description:
"Open new terminals in the current tab's directory when possible",
keywords: [
"terminal",
"directory",
"cwd",
"current working directory",
"new terminal",
"follow tab",
"open here",
"folder",
"path",
],
},
{
id: SETTING_ITEM_ID.TERMINAL_SESSIONS,
section: "terminal",
Expand Down
42 changes: 37 additions & 5 deletions apps/desktop/src/renderer/stores/tabs/useTabsWithPresets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@ import {
} from "./preset-launch";
import { useTabsStore } from "./store";
import type { AddTabOptions, SplitPaneOptions } from "./types";
import { resolveActiveTabIdForWorkspace } from "./utils";
import {
resolveActiveTabIdForWorkspace,
resolveNewTerminalInitialCwd,
} from "./utils";

interface OpenPresetOptions {
target?: PresetOpenTarget;
modeOverride?: PresetMode;
fallbackInitialCwd?: string;
}

interface PreparedPreset {
Expand Down Expand Up @@ -51,6 +55,9 @@ function preparePreset(preset: TerminalPreset): PreparedPreset {
export function useTabsWithPresets() {
const { data: newTabPresets = [] } =
electronTrpc.settings.getNewTabPresets.useQuery();
const { data: openInCurrentTabDirectory = true } =
electronTrpc.settings.getNewTerminalInCurrentTabDirectory.useQuery();
const utils = electronTrpc.useUtils();

const storeAddTab = useTabsStore((s) => s.addTab);
const storeAddTabWithMultiplePanes = useTabsStore(
Expand Down Expand Up @@ -350,7 +357,13 @@ export function useTabsWithPresets() {
const prepared = preparePreset(preset);
const target = options?.target ?? "new-tab";
const mode = options?.modeOverride ?? prepared.mode;
return executePreset(workspaceId, { ...prepared, mode }, target);
const initialCwd =
prepared.initialCwd ?? options?.fallbackInitialCwd ?? undefined;
return executePreset(
workspaceId,
{ ...prepared, mode, initialCwd },
target,
);
},
[executePreset],
);
Expand All @@ -361,20 +374,39 @@ export function useTabsWithPresets() {
return storeAddTab(workspaceId, options);
}

const sourceInitialCwd = openInCurrentTabDirectory
? resolveNewTerminalInitialCwd({
workspaceId,
worktreePath: utils.workspaces.get.getData({ id: workspaceId })
?.worktreePath,
tabs: useTabsStore.getState().tabs,
panes: useTabsStore.getState().panes,
activeTabIds: useTabsStore.getState().activeTabIds,
tabHistoryStacks: useTabsStore.getState().tabHistoryStacks,
focusedPaneIds: useTabsStore.getState().focusedPaneIds,
})
: undefined;

if (newTabPresets.length === 0) {
return storeAddTab(workspaceId);
return storeAddTab(workspaceId, {
initialCwd: sourceInitialCwd,
});
}

const firstResult = openPreset(workspaceId, newTabPresets[0], {
target: "new-tab",
fallbackInitialCwd: sourceInitialCwd,
});
for (let i = 1; i < newTabPresets.length; i++) {
openPreset(workspaceId, newTabPresets[i], { target: "new-tab" });
openPreset(workspaceId, newTabPresets[i], {
target: "new-tab",
fallbackInitialCwd: sourceInitialCwd,
});
}

return { tabId: firstResult.tabId, paneId: firstResult.paneId };
},
[storeAddTab, newTabPresets, openPreset],
[storeAddTab, newTabPresets, openInCurrentTabDirectory, openPreset, utils],
);

const addPane = useCallback(
Expand Down
87 changes: 87 additions & 0 deletions apps/desktop/src/renderer/stores/tabs/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
getAdjacentPaneId,
resolveActiveTabIdForWorkspace,
resolveFileViewerMode,
resolveNewTerminalInitialCwd,
} from "./utils";

describe("findPanePath", () => {
Expand Down Expand Up @@ -403,6 +404,92 @@ describe("buildMultiPaneLayout", () => {
});
});

describe("resolveNewTerminalInitialCwd", () => {
const createTab = ({
id,
workspaceId,
}: {
id: string;
workspaceId: string;
}): Tab => ({
id,
name: id,
workspaceId,
layout: `${id}-pane`,
createdAt: 0,
});

it("uses the focused terminal cwd when available", () => {
expect(
resolveNewTerminalInitialCwd({
workspaceId: "ws-1",
worktreePath: "/repo",
tabs: [createTab({ id: "tab-1", workspaceId: "ws-1" })],
panes: {
"pane-1": {
id: "pane-1",
tabId: "tab-1",
type: "terminal",
name: "Terminal",
cwd: "/repo/packages/ui",
},
},
activeTabIds: { "ws-1": "tab-1" },
tabHistoryStacks: {},
focusedPaneIds: { "tab-1": "pane-1" },
}),
).toBe("/repo/packages/ui");
});

it("falls back to the file viewer's directory", () => {
expect(
resolveNewTerminalInitialCwd({
workspaceId: "ws-1",
worktreePath: "/repo",
tabs: [createTab({ id: "tab-1", workspaceId: "ws-1" })],
panes: {
"pane-1": {
id: "pane-1",
tabId: "tab-1",
type: "file-viewer",
name: "Button.tsx",
fileViewer: {
filePath: "packages/ui/src/button.tsx",
viewMode: "raw",
isPinned: false,
diffLayout: "inline",
},
},
},
activeTabIds: { "ws-1": "tab-1" },
tabHistoryStacks: {},
focusedPaneIds: { "tab-1": "pane-1" },
}),
).toBe("/repo/packages/ui/src");
});

it("falls back to the workspace root when no directory can be resolved", () => {
expect(
resolveNewTerminalInitialCwd({
workspaceId: "ws-1",
worktreePath: "/repo",
tabs: [createTab({ id: "tab-1", workspaceId: "ws-1" })],
panes: {
"pane-1": {
id: "pane-1",
tabId: "tab-1",
type: "webview",
name: "Browser",
},
},
activeTabIds: { "ws-1": "tab-1" },
tabHistoryStacks: {},
focusedPaneIds: { "tab-1": "pane-1" },
}),
).toBe("/repo");
Comment on lines +422 to +489
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Locate the implementation of resolveNewTerminalInitialCwd
rg -l "resolveNewTerminalInitialCwd" --type ts --type tsx

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

# Get details about the function implementation
rg -A 30 "function resolveNewTerminalInitialCwd|export.*resolveNewTerminalInitialCwd" --type ts --type tsx

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

# Find the test file and implementation
fd -e ts -e tsx | grep -E "(utils\.test|utils\.ts).*tabs"

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Search for the function without file type constraints
rg "resolveNewTerminalInitialCwd" -A 30

Repository: superset-sh/superset

Length of output: 17355


🏁 Script executed:

# Get the full implementation of resolveNewTerminalInitialCwd
cat apps/desktop/src/renderer/stores/tabs/utils.ts | head -200 | tail -100

Repository: superset-sh/superset

Length of output: 2479


🏁 Script executed:

# Let's search for the function and get more lines
rg -A 50 "export function resolveNewTerminalInitialCwd" apps/desktop/src/renderer/stores/tabs/utils.ts

Repository: superset-sh/superset

Length of output: 1159


🏁 Script executed:

# Check the imports at the top of utils.ts
head -30 apps/desktop/src/renderer/stores/tabs/utils.ts

Repository: superset-sh/superset

Length of output: 878


🏁 Script executed:

# Check if there's any platform-specific path handling elsewhere
rg "process.platform|win32|darwin|linux" apps/desktop/src/renderer/stores/tabs/

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Look at the test file to confirm the exact line numbers match the review
sed -n '420,495p' apps/desktop/src/renderer/stores/tabs/utils.test.ts

Repository: superset-sh/superset

Length of output: 1900


🏁 Script executed:

# Double-check: does test 2 actually use the implementation path with join/dirname?
sed -n '453,467p' apps/desktop/src/renderer/stores/tabs/utils.test.ts

Repository: superset-sh/superset

Length of output: 421


🏁 Script executed:

# Verify: on what platform would these tests be run?
# Check if there's any CI/testing setup
fd -name "*.yml" -o -name "*.yaml" | grep -E "(github|gitlab|ci|test)" | head -5

Repository: superset-sh/superset

Length of output: 294


🏁 Script executed:

# Check for CI configuration files
fd -e yml -e yaml | grep -i -E "(github|gitlab|ci|test)"

Repository: superset-sh/superset

Length of output: 46


🏁 Script executed:

# Also check package.json to see what test command is used
cat apps/desktop/package.json | grep -A 5 "test"

Repository: superset-sh/superset

Length of output: 200


Make these CWD assertions platform-neutral.

These test cases hard-code POSIX paths, but the file-viewer branch uses dirname(join(worktreePath, pane.fileViewer.filePath)) which is platform-dependent. On Windows, path separators become backslashes, causing assertions to fail.

Suggested test adjustment
+import { dirname, join } from "node:path";
 import { describe, expect, it } from "bun:test";
 import type { MosaicNode } from "react-mosaic-component";
 import type { Tab } from "./types";
 import {
 	buildMultiPaneLayout,
@@
 describe("resolveNewTerminalInitialCwd", () => {
+	const repoRoot = join("/", "repo");
+
 	const createTab = ({
 		id,
 		workspaceId,
 	}: {
@@
 			resolveNewTerminalInitialCwd({
 				workspaceId: "ws-1",
-				worktreePath: "/repo",
+				worktreePath: repoRoot,
 				tabs: [createTab({ id: "tab-1", workspaceId: "ws-1" })],
 				panes: {
 					"pane-1": {
@@
-						cwd: "/repo/packages/ui",
+						cwd: join(repoRoot, "packages", "ui"),
 					},
 				},
@@
-		).toBe("/repo/packages/ui");
+		).toBe(join(repoRoot, "packages", "ui"));
 	});
@@
 			resolveNewTerminalInitialCwd({
 				workspaceId: "ws-1",
-				worktreePath: "/repo",
+				worktreePath: repoRoot,
@@
-		).toBe("/repo/packages/ui/src");
+		).toBe(
+			dirname(join(repoRoot, "packages", "ui", "src", "button.tsx")),
+		);
 	});
@@
 			resolveNewTerminalInitialCwd({
 				workspaceId: "ws-1",
-				worktreePath: "/repo",
+				worktreePath: repoRoot,
@@
-		).toBe("/repo");
+		).toBe(repoRoot);
 	});
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/stores/tabs/utils.test.ts` around lines 422 - 489,
Tests hard-code POSIX paths causing Windows failures; update the three
assertions around resolveNewTerminalInitialCwd to build platform-neutral
expected values using Node's path utilities: import path and for the focused
terminal case call path.normalize("/repo/packages/ui") or path.join parts, for
the file-viewer case compute expected via path.dirname(path.join("/repo",
"packages/ui/src/button.tsx")) (or using the same worktreePath and
pane.fileViewer.filePath values from the test) and for the fallback case use
path.normalize(worktreePath) or path.join(...) so assertions compare
path-normalized values instead of hard-coded POSIX strings; reference
resolveNewTerminalInitialCwd, pane.fileViewer.filePath, and worktreePath to
locate where to change the expected values.

});
});

describe("resolveFileViewerMode", () => {
it("returns diff for modified file with diffCategory", () => {
expect(
Expand Down
Loading
Loading