-
Notifications
You must be signed in to change notification settings - Fork 402
Deleting current workspace switches to another instead of closing [ backend implementation ] #1623
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
ce30fcb
a68696b
9edfe9a
e87e98f
e87d166
c423d81
2cc2241
2d3dca6
8c10180
a146bc6
7933f7e
a64a1b1
267ebdf
190ff4b
59b8d28
c8139f9
8275de3
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 |
---|---|---|
|
@@ -66,7 +66,8 @@ func CreateWorkspace(ctx context.Context, name string, icon string, color string | |
} | ||
|
||
wps.Broker.Publish(wps.WaveEvent{ | ||
Event: wps.Event_WorkspaceUpdate}) | ||
Event: wps.Event_WorkspaceUpdate, | ||
}) | ||
|
||
ws, _, err = UpdateWorkspace(ctx, ws.OID, name, icon, color, applyDefaults) | ||
return ws, err | ||
|
@@ -114,40 +115,74 @@ func UpdateWorkspace(ctx context.Context, workspaceId string, name string, icon | |
// If force is true, it will delete even if workspace is named. | ||
// If workspace is empty, it will be deleted, even if it is named. | ||
// Returns true if workspace was deleted, false if it was not deleted. | ||
func DeleteWorkspace(ctx context.Context, workspaceId string, force bool) (bool, error) { | ||
func DeleteWorkspace(ctx context.Context, workspaceId string, force bool) (bool, string, error) { | ||
log.Printf("DeleteWorkspace %s\n", workspaceId) | ||
workspace, err := wstore.DBMustGet[*waveobj.Workspace](ctx, workspaceId) | ||
if err != nil && wstore.ErrNotFound == err { | ||
return true, "", fmt.Errorf("workspace already deleted %w", err) | ||
} | ||
// @jalileh list needs to be saved early on i assume | ||
workspaces, err := ListWorkspaces(ctx) | ||
if err != nil { | ||
return false, "", fmt.Errorf("error retrieving workspaceList: %w", err) | ||
} | ||
|
||
if err != nil { | ||
return false, fmt.Errorf("error getting workspace: %w", err) | ||
return false, "", fmt.Errorf("error getting workspace: %w", err) | ||
} | ||
if workspace.Name != "" && workspace.Icon != "" && !force && (len(workspace.TabIds) > 0 || len(workspace.PinnedTabIds) > 0) { | ||
log.Printf("Ignoring DeleteWorkspace for workspace %s as it is named\n", workspaceId) | ||
return false, nil | ||
return false, "", nil | ||
} | ||
|
||
// delete all pinned and unpinned tabs | ||
for _, tabId := range append(workspace.TabIds, workspace.PinnedTabIds...) { | ||
log.Printf("deleting tab %s\n", tabId) | ||
_, err := DeleteTab(ctx, workspaceId, tabId, false) | ||
if err != nil { | ||
return false, fmt.Errorf("error closing tab: %w", err) | ||
return false, "", fmt.Errorf("error closing tab: %w", err) | ||
} | ||
} | ||
windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, workspaceId) | ||
err = wstore.DBDelete(ctx, waveobj.OType_Workspace, workspaceId) | ||
if err != nil { | ||
return false, fmt.Errorf("error deleting workspace: %w", err) | ||
return false, "", fmt.Errorf("error deleting workspace: %w", err) | ||
Jalileh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
log.Printf("deleted workspace %s\n", workspaceId) | ||
wps.Broker.Publish(wps.WaveEvent{ | ||
Event: wps.Event_WorkspaceUpdate}) | ||
Event: wps.Event_WorkspaceUpdate, | ||
}) | ||
|
||
if windowId != "" { | ||
err = CloseWindow(ctx, windowId, false) | ||
|
||
UnclaimedWorkspace, findAfter := "", false | ||
for _, ws := range workspaces { | ||
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. I liked your old logic which would attempt to find the workspace at index closest to the one that was deleted, could you incorporate that here? I think you can do this in one pass. Create three variables, 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. 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. Ah it runs in switch workspace to clean up unsaved workspaces. For saved ones, delete without a force flag is a no-op 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. how do I actually retrieve the error string in the error data structure? if I can get the string to reliably know if 'not found' happened I wont have to limit the claimworkspace implementation, I only saw msg inside of it but couldn't access it, sorry I'm new to go. 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. I think you can return empty string if nothing is deleted. This should never happen if you call deleteworkspace from the workspace service since that passes the force flag as true to the wcore implementation 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. OK ill implement the old logic tomorrow and notify you, ty! |
||
if ws.WorkspaceId == workspaceId { | ||
if UnclaimedWorkspace != "" { | ||
break | ||
} | ||
findAfter = true | ||
continue | ||
} | ||
if findAfter && ws.WindowId == "" { | ||
UnclaimedWorkspace = ws.WorkspaceId | ||
break | ||
} else if ws.WindowId == "" { | ||
UnclaimedWorkspace = ws.WorkspaceId | ||
} | ||
} | ||
|
||
if UnclaimedWorkspace != "" { | ||
return true, UnclaimedWorkspace, nil | ||
} else { | ||
err = CloseWindow(ctx, windowId, false) | ||
} | ||
|
||
if err != nil { | ||
return false, fmt.Errorf("error closing window: %w", err) | ||
return false, "", fmt.Errorf("error closing window: %w", err) | ||
} | ||
} | ||
return true, nil | ||
return true, "", nil | ||
} | ||
|
||
func GetWorkspace(ctx context.Context, wsID string) (*waveobj.Workspace, error) { | ||
|
Uh oh!
There was an error while loading. Please reload this page.