Skip to content
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

[debug] The debug view does not store the selected launch config. #7918

Closed
kittaakos opened this issue May 28, 2020 · 2 comments · Fixed by #10287
Closed

[debug] The debug view does not store the selected launch config. #7918

kittaakos opened this issue May 28, 2020 · 2 comments · Fixed by #10287
Labels
bug bugs found in the application debug issues that related to debug functionality help wanted issues meant to be picked up, require help

Comments

@kittaakos
Copy link
Contributor

Bug Description:

As the title says, it always shows the first item in the list after a browser refresh: "Attach by process ID"

Steps to Reproduce:

  1. Select "Launch with Node.js" launch config,
  2. Refresh the browser,
  3. It is "Attach by process ID"

Additional Information

  • Operating System:
  • Theia Version:
@vince-fugnitto vince-fugnitto added bug bugs found in the application debug issues that related to debug functionality labels May 28, 2020
@akosyakov akosyakov added the help wanted issues meant to be picked up, require help label May 29, 2020
@kittaakos
Copy link
Contributor Author

We cannot use the StatefulWidget hooks out of the box for restoration as the launch configs are loaded asynchronously; when we're restoring the layout, the DebugConfigurationManager does not know any configs; it's not possible to set the current. I came up with this, it works, it is definitely better than nothing but I do not like this approach much. Other suggestions?

diff --git a/packages/debug/src/browser/view/debug-configuration-widget.tsx b/packages/debug/src/browser/view/debug-configuration-widget.tsx
index 5791f55af..0344dab3d 100644
--- a/packages/debug/src/browser/view/debug-configuration-widget.tsx
+++ b/packages/debug/src/browser/view/debug-configuration-widget.tsx
@@ -18,7 +18,7 @@ import * as React from 'react';
 import { injectable, inject, postConstruct } from 'inversify';
 import { Disposable } from '@theia/core/lib/common';
 import URI from '@theia/core/lib/common/uri';
-import { ReactWidget } from '@theia/core/lib/browser';
+import { ReactWidget, StatefulWidget } from '@theia/core/lib/browser';
 import { WorkspaceService } from '@theia/workspace/lib/browser';
 import { DebugConsoleContribution } from '../console/debug-console-contribution';
 import { DebugConfigurationManager } from '../debug-configuration-manager';
@@ -30,7 +30,7 @@ import { DebugCommands } from '../debug-frontend-application-contribution';
 import { CommandRegistry } from '@theia/core/lib/common';
 
 @injectable()
-export class DebugConfigurationWidget extends ReactWidget {
+export class DebugConfigurationWidget extends ReactWidget implements StatefulWidget {
 
     @inject(CommandRegistry)
     protected readonly commandRegistry: CommandRegistry;
@@ -50,6 +50,8 @@ export class DebugConfigurationWidget extends ReactWidget {
     @inject(WorkspaceService)
     protected readonly workspaceService: WorkspaceService;
 
+    protected stateToRestore?: DebugConfigurationWidget.State;
+
     @postConstruct()
     protected init(): void {
         this.addClass('debug-toolbar');
@@ -90,6 +92,15 @@ export class DebugConfigurationWidget extends ReactWidget {
         </React.Fragment>;
     }
     protected get currentValue(): string {
+        if (this.stateToRestore?.selectedConfiguration) {
+            const [name, workspaceFolderUri] = this.stateToRestore.selectedConfiguration.split('__CONF__');
+            const config = this.manager.find(name, workspaceFolderUri);
+            if (config) {
+                this.stateToRestore = undefined;
+                this.manager.current = config;
+                return this.toValue(config);
+            }
+        }
         const { current } = this.manager;
         return current ? this.toValue(current) : '__NO_CONF__';
     }
@@ -131,4 +142,34 @@ export class DebugConfigurationWidget extends ReactWidget {
         activate: true
     });
 
+    storeState(): DebugConfigurationWidget.State {
+        const { current } = this.manager;
+        if (current) {
+            return {
+                selectedConfiguration: this.toValue(current)
+            };
+        }
+        return {};
+    }
+
+    restoreState(state?: Partial<DebugConfigurationWidget.State>): void {
+        if (state?.selectedConfiguration) {
+            const [name, workspaceFolderUri] = state?.selectedConfiguration.split('__CONF__');
+            const config = this.manager.find(name, workspaceFolderUri);
+            if (config) {
+                this.manager.current = config;
+            } else {
+                // Launch/Debug configs are loaded asynchronously, at the widget restoration phase they might not be available.
+                if (Array.from(this.manager.all)) {
+                    this.stateToRestore = state;
+                }
+            }
+        }
+    }
+
+}
+export namespace DebugConfigurationWidget {
+    export interface State {
+        selectedConfiguration?: string;
+    }
 }
diff --git a/packages/debug/src/browser/view/debug-widget.ts b/packages/debug/src/browser/view/debug-widget.ts
index 4be43613d..08f8a89b3 100644
--- a/packages/debug/src/browser/view/debug-widget.ts
+++ b/packages/debug/src/browser/view/debug-widget.ts
@@ -90,11 +90,15 @@ export class DebugWidget extends BaseWidget implements StatefulWidget, Applicati
     }
 
     storeState(): object {
-        return this.sessionWidget.storeState();
+        return {
+            ...this.sessionWidget.storeState(),
+            ...this.toolbar.storeState()
+        };
     }
 
-    restoreState(oldState: ViewContainer.State): void {
+    restoreState(oldState: ViewContainer.State & Partial<DebugConfigurationWidget.State>): void {
         this.sessionWidget.restoreState(oldState);
+        this.toolbar.restoreState(oldState);
     }
 
 }

@kittaakos
Copy link
Contributor Author

Or we need an event on the config manager (ready or something) and we do the restoration on that event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application debug issues that related to debug functionality help wanted issues meant to be picked up, require help
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants