-
Notifications
You must be signed in to change notification settings - Fork 965
feat(desktop): show ahead/behind status on local workspace #1496
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ae86d35
ec485ed
1bf030e
5b4256a
6088b17
1470fe2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1090,6 +1090,31 @@ export async function checkNeedsRebase( | |
| return Number.parseInt(behindCount.trim(), 10) > 0; | ||
| } | ||
|
|
||
| export async function getAheadBehindCount({ | ||
| repoPath, | ||
| defaultBranch, | ||
| }: { | ||
| repoPath: string; | ||
| defaultBranch: string; | ||
| }): Promise<{ ahead: number; behind: number }> { | ||
| const git = simpleGit(repoPath); | ||
| try { | ||
| const output = await git.raw([ | ||
| "rev-list", | ||
| "--left-right", | ||
| "--count", | ||
| `origin/${defaultBranch}...HEAD`, | ||
| ]); | ||
| const [behindStr, aheadStr] = output.trim().split(/\s+/); | ||
| return { | ||
| ahead: Number.parseInt(aheadStr || "0", 10), | ||
| behind: Number.parseInt(behindStr || "0", 10), | ||
| }; | ||
| } catch { | ||
| return { ahead: 0, behind: 0 }; | ||
| } | ||
| } | ||
|
Comment on lines
+1093
to
+1116
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for all references to checkNeedsRebase in the codebase
rg -n 'checkNeedsRebase' --type=ts --type=tsxRepository: superset-sh/superset Length of output: 91 🏁 Script executed: # Check the git.ts file for `any` type usage and other potential issues
head -100 apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsRepository: superset-sh/superset Length of output: 2626 🏁 Script executed: # Check if the file uses proper aliases as defined in tsconfig
grep -n "import.*from" apps/desktop/src/lib/trpc/routers/workspaces/utils/git.ts | head -20Repository: superset-sh/superset Length of output: 612 🏁 Script executed: # Search for all references to checkNeedsRebase without file type restriction
rg -n 'checkNeedsRebase' apps/desktop/src/lib/trpc/routers/workspaces/utils/Repository: superset-sh/superset Length of output: 166 🏁 Script executed: # Read lines 1070-1120 to see checkNeedsRebase and the new function in context
sed -n '1070,1120p' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsRepository: superset-sh/superset Length of output: 1144 🏁 Script executed: # Search for any type usage in git.ts
rg -n ': any' apps/desktop/src/lib/trpc/routers/workspaces/utils/git.tsRepository: superset-sh/superset Length of output: 46 🏁 Script executed: # Check if checkNeedsRebase is exported from git.ts and imported elsewhere
rg -n 'export.*checkNeedsRebase|import.*checkNeedsRebase' apps/desktop/Repository: superset-sh/superset Length of output: 166 🏁 Script executed: # Search entire repository for checkNeedsRebase imports/usage (not just definition)
rg -n 'checkNeedsRebase' --type tsRepository: superset-sh/superset Length of output: 166 🏁 Script executed: # Specifically check git-status.ts mentioned in the review
find apps/desktop -name "git-status.ts" -o -name "*git-status*" | head -5Repository: superset-sh/superset Length of output: 133 🏁 Script executed: # Look for any references to checkNeedsRebase in the whole repo
rg 'checkNeedsRebase' -g '!git.ts'Repository: superset-sh/superset Length of output: 46 🏁 Script executed: # Check git-status.ts to see how getAheadBehindCount is used
cat -n apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.ts | head -50Repository: superset-sh/superset Length of output: 1817 🏁 Script executed: # Also search git-status.ts for any references to checkNeedsRebase or needsRebase
rg -n 'needsRebase|checkNeedsRebase' apps/desktop/src/lib/trpc/routers/workspaces/procedures/git-status.tsRepository: superset-sh/superset Length of output: 96 Remove the The The existing 🤖 Prompt for AI Agents |
||
|
|
||
| export async function hasUncommittedChanges( | ||
| worktreePath: string, | ||
| ): Promise<boolean> { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| import { | ||
| ContextMenu, | ||
| ContextMenuContent, | ||
| ContextMenuItem, | ||
| ContextMenuSeparator, | ||
| ContextMenuTrigger, | ||
| } from "@superset/ui/context-menu"; | ||
| import { | ||
| HoverCard, | ||
| HoverCardContent, | ||
| HoverCardTrigger, | ||
| } from "@superset/ui/hover-card"; | ||
| import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip"; | ||
| import { cn } from "@superset/ui/utils"; | ||
| import type { RefObject } from "react"; | ||
| import { LuCopy, LuX } from "react-icons/lu"; | ||
| import type { ActivePaneStatus } from "shared/tabs-types"; | ||
| import { STROKE_WIDTH } from "../constants"; | ||
| import { DeleteWorkspaceDialog, WorkspaceHoverCardContent } from "./components"; | ||
| import { HOVER_CARD_CLOSE_DELAY, HOVER_CARD_OPEN_DELAY } from "./constants"; | ||
| import { WorkspaceIcon } from "./WorkspaceIcon"; | ||
|
|
||
| interface CollapsedWorkspaceItemProps { | ||
| id: string; | ||
| name: string; | ||
| branch: string; | ||
| type: "worktree" | "branch"; | ||
| isActive: boolean; | ||
| isUnread: boolean; | ||
| workspaceStatus: ActivePaneStatus | null; | ||
| itemRef: RefObject<HTMLElement | null>; | ||
| showDeleteDialog: boolean; | ||
| setShowDeleteDialog: (open: boolean) => void; | ||
| onMouseEnter: () => void; | ||
| onClick: () => void; | ||
| onDeleteClick: () => void; | ||
| onCopyPath: () => void; | ||
| } | ||
|
|
||
| export function CollapsedWorkspaceItem({ | ||
| id, | ||
| name, | ||
| branch, | ||
| type, | ||
| isActive, | ||
| isUnread, | ||
| workspaceStatus, | ||
| itemRef, | ||
| showDeleteDialog, | ||
| setShowDeleteDialog, | ||
| onMouseEnter, | ||
| onClick, | ||
| onDeleteClick, | ||
| onCopyPath, | ||
| }: CollapsedWorkspaceItemProps) { | ||
| const isBranchWorkspace = type === "branch"; | ||
|
|
||
| const collapsedButton = ( | ||
| <button | ||
| ref={(node) => { | ||
| (itemRef as React.MutableRefObject<HTMLElement | null>).current = node; | ||
| }} | ||
| type="button" | ||
| onClick={onClick} | ||
| onMouseEnter={onMouseEnter} | ||
| className={cn( | ||
| "relative flex items-center justify-center size-8 rounded-md", | ||
| "hover:bg-muted/50 transition-colors", | ||
| isActive && "bg-muted", | ||
| )} | ||
| > | ||
| <WorkspaceIcon | ||
| isBranchWorkspace={isBranchWorkspace} | ||
| isActive={isActive} | ||
| isUnread={isUnread} | ||
| workspaceStatus={workspaceStatus} | ||
| variant="collapsed" | ||
| /> | ||
| </button> | ||
| ); | ||
|
|
||
| if (isBranchWorkspace) { | ||
| return ( | ||
| <Tooltip delayDuration={300}> | ||
| <TooltipTrigger asChild>{collapsedButton}</TooltipTrigger> | ||
| <TooltipContent side="right" className="flex flex-col gap-0.5"> | ||
| <span className="font-medium">local</span> | ||
| <span className="text-xs text-muted-foreground font-mono"> | ||
| {branch} | ||
| </span> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <HoverCard | ||
| openDelay={HOVER_CARD_OPEN_DELAY} | ||
| closeDelay={HOVER_CARD_CLOSE_DELAY} | ||
| > | ||
| <ContextMenu> | ||
| <HoverCardTrigger asChild> | ||
| <ContextMenuTrigger asChild>{collapsedButton}</ContextMenuTrigger> | ||
| </HoverCardTrigger> | ||
| <ContextMenuContent> | ||
| <ContextMenuItem onSelect={onCopyPath}> | ||
| <LuCopy className="size-4 mr-2" strokeWidth={STROKE_WIDTH} /> | ||
| Copy Path | ||
| </ContextMenuItem> | ||
| <ContextMenuSeparator /> | ||
| <ContextMenuItem onSelect={() => onDeleteClick()}> | ||
| <LuX className="size-4 mr-2" strokeWidth={STROKE_WIDTH} /> | ||
| Close Worktree | ||
| </ContextMenuItem> | ||
| </ContextMenuContent> | ||
| </ContextMenu> | ||
| <HoverCardContent side="right" align="start" className="w-72"> | ||
| <WorkspaceHoverCardContent workspaceId={id} workspaceAlias={name} /> | ||
| </HoverCardContent> | ||
| </HoverCard> | ||
| <DeleteWorkspaceDialog | ||
| workspaceId={id} | ||
| workspaceName={name} | ||
| workspaceType={type} | ||
| open={showDeleteDialog} | ||
| onOpenChange={setShowDeleteDialog} | ||
| /> | ||
| </> | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| interface WorkspaceAheadBehindProps { | ||
| ahead: number; | ||
| behind: number; | ||
| } | ||
|
|
||
| export function WorkspaceAheadBehind({ | ||
| ahead, | ||
| behind, | ||
| }: WorkspaceAheadBehindProps) { | ||
| if (ahead === 0 && behind === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return ( | ||
| <div className="flex items-center gap-1.5 text-[10px] font-mono tabular-nums shrink-0"> | ||
| {behind > 0 && <span className="text-amber-500">↓{behind}</span>} | ||
| {ahead > 0 && <span className="text-emerald-500">↑{ahead}</span>} | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify that
getAheadBehindsemantics match the intended use for branch workspaces.The procedure compares
origin/{workspace.branch}...HEADin the main repo. This correctly shows how the local branch relates to its remote tracking branch — but only when the main repo's HEAD is onworkspace.branch. If the user switches branches externally (e.g., via terminal),HEADwill refer to a different branch and the numbers will be misleading.Consider using an explicit branch ref instead of implicit
HEADto make this resilient:Proposed fix
In
getAheadBehindCount(or a variant), replaceHEADwith the explicit branch name:This would require either a separate parameter or passing the local branch name explicitly. The tradeoff is that
refreshGitStatus(for worktrees) correctly usesHEADsince the worktree's HEAD is always on its branch.🤖 Prompt for AI Agents