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.

Sign up for GitHub

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

Add support for compound launches #11444

Merged
merged 12 commits into from
Aug 11, 2022
Merged

Add support for compound launches #11444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9a7361f
Add support for compound launches
martin-fleck-at Jul 20, 2022
8117512
Incorporate review
martin-fleck-at Jul 21, 2022
c3908a1
Incorporate review
martin-fleck-at Jul 22, 2022
593619d
Fix inproper refresh of debug configuration selection
martin-fleck-at Jul 26, 2022
e059a8c
Integrate further feedback
martin-fleck-at Jul 28, 2022
b96e332
Fix missing update of current configuration, need to search
martin-fleck-at Jul 28, 2022
45caa7d
Fix type guard of session options
martin-fleck-at Jul 28, 2022
375f093
Fix missing update of current configuration, need to search better
martin-fleck-at Jul 28, 2022
5a8d61e
Fix name calculation of stored dynamic session options
martin-fleck-at Jul 29, 2022
efb090d
Fix select rendering and 'Add Configurations' in multi-root workspace
martin-fleck-at Aug 1, 2022
038423d
Fix incomplete restoring of old data
martin-fleck-at Aug 2, 2022
fc34582
Ensure dynamic options are properly restored as 'current'
martin-fleck-at Aug 4, 2022
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
32 changes: 13 additions & 19 deletions packages/core/src/browser/widgets/select-component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface SelectOption {
detail?: string
description?: string
markdown?: boolean
userData?: string
}

export interface SelectComponentProps {
Expand All @@ -40,15 +41,8 @@ export interface SelectComponentProps {
onFocus?: () => void
}

export interface SelectComponentDropdownDimensions {
top: number
left: number
width: number
parentHeight: number
};

export interface SelectComponentState {
dimensions?: SelectComponentDropdownDimensions
dimensions?: DOMRect
selected: number
original: number
hover: number
Expand Down Expand Up @@ -86,6 +80,10 @@ export class SelectComponent extends React.Component<SelectComponentProps, Selec
this.dropdownElement = list;
}

get options(): SelectOption[] {
return this.props.options;
}

get value(): string | number | undefined {
return this.props.options[this.state.selected].value ?? this.state.selected;
}
Expand Down Expand Up @@ -258,14 +256,7 @@ export class SelectComponent extends React.Component<SelectComponentProps, Selec
}
if (!this.state.dimensions) {
const rect = this.fieldRef.current.getBoundingClientRect();
this.setState({
dimensions: {
top: rect.top + rect.height,
left: rect.left,
width: rect.width,
parentHeight: rect.height
},
});
this.setState({ dimensions: rect });
} else {
this.hide();
}
Expand Down Expand Up @@ -293,7 +284,10 @@ export class SelectComponent extends React.Component<SelectComponentProps, Selec
this.optimalHeight = this.getOptimalHeight(Math.max(this.state.dimensions.width, this.optimalWidth));
}
const clientRect = document.getElementById('theia-app-shell')!.getBoundingClientRect();
const invert = this.optimalHeight > clientRect.height - this.state.dimensions.top;
const availableTop = this.state.dimensions.top - clientRect.top;
const availableBottom = clientRect.top + clientRect.height - this.state.dimensions.bottom;
// prefer rendering to the bottom unless there is not enough space and more content can be shown to the top
const invert = availableBottom < this.optimalHeight && (availableBottom - this.optimalHeight) < (availableTop - this.optimalHeight);
const { options } = this.props;
const { hover } = this.state;
const description = options[hover].description;
Expand All @@ -319,8 +313,8 @@ export class SelectComponent extends React.Component<SelectComponentProps, Selec
const calculatedWidth = Math.max(this.state.dimensions.width, this.optimalWidth);
const maxWidth = clientRect.width - this.state.dimensions.left;
return <div key="dropdown" className="theia-select-component-dropdown" style={{
top: invert ? 'none' : this.state.dimensions.top,
bottom: invert ? clientRect.height - this.state.dimensions.top + this.state.dimensions.parentHeight : 'none',
top: invert ? 'none' : this.state.dimensions.bottom,
bottom: invert ? clientRect.top + clientRect.height - this.state.dimensions.top : 'none',
left: this.state.dimensions.left,
width: Math.min(calculatedWidth, maxWidth),
position: 'absolute'
Expand Down
132 changes: 92 additions & 40 deletions packages/debug/src/browser/debug-configuration-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { LabelProvider, PreferenceScope, PreferenceService, QuickPickValue, Stor
import { QuickPickService } from '@theia/core/lib/common/quick-pick-service';
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';
import { DebugConfigurationModel } from './debug-configuration-model';
import { DebugSessionOptions } from './debug-session-options';
import { DebugSessionOptions, DynamicDebugConfigurationSessionOptions } from './debug-session-options';
import { DebugService } from '../common/debug-service';
import { ContextKey, ContextKeyService } from '@theia/core/lib/browser/context-key-service';
import { DebugConfiguration } from '../common/debug-common';
Expand All @@ -41,6 +41,7 @@ import * as monaco from '@theia/monaco-editor-core';
import { ICommandService } from '@theia/monaco-editor-core/esm/vs/platform/commands/common/commands';
import { StandaloneServices } from '@theia/monaco-editor-core/esm/vs/editor/standalone/browser/standaloneServices';
import { nls } from '@theia/core';
import { DebugCompound } from '../common/debug-compound';

export interface WillProvideDebugConfiguration extends WaitUntilEvent {
}
Expand Down Expand Up @@ -94,7 +95,7 @@ export class DebugConfigurationManager {

protected initialized: Promise<void>;

protected recentDynamicOptionsTracker: DebugSessionOptions[] = [];
protected recentDynamicOptionsTracker: DynamicDebugConfigurationSessionOptions[] = [];

@postConstruct()
protected async init(): Promise<void> {
Expand Down Expand Up @@ -141,10 +142,10 @@ export class DebugConfigurationManager {
protected *getAll(): IterableIterator<DebugSessionOptions> {
for (const model of this.models.values()) {
for (const configuration of model.configurations) {
yield {
configuration,
workspaceFolderUri: model.workspaceFolderUri
};
yield this.configurationToOptions(configuration, model.workspaceFolderUri);
}
for (const compound of model.compounds) {
yield this.compoundToOptions(compound, model.workspaceFolderUri);
}
}
}
Expand All @@ -159,7 +160,7 @@ export class DebugConfigurationManager {
}
protected *doGetSupported(debugTypes: Set<string>): IterableIterator<DebugSessionOptions> {
for (const options of this.getAll()) {
if (debugTypes.has(options.configuration.type)) {
if (options.configuration && debugTypes.has(options.configuration.type)) {
yield options;
}
}
Expand All @@ -171,8 +172,7 @@ export class DebugConfigurationManager {
}

async getSelectedConfiguration(): Promise<DebugSessionOptions | undefined> {
// providerType applies to dynamic configurations only
if (!this._currentOptions?.providerType) {
if (!DebugSessionOptions.isDynamic(this._currentOptions)) {
return this._currentOptions;
}

Expand All @@ -188,7 +188,7 @@ export class DebugConfigurationManager {
throw new Error(message);
}

return { configuration, providerType };
return { name, configuration, providerType };
}

set current(option: DebugSessionOptions | undefined) {
Expand All @@ -197,7 +197,7 @@ export class DebugConfigurationManager {
}

protected updateRecentlyUsedDynamicConfigurationOptions(option: DebugSessionOptions | undefined): void {
if (option?.providerType) { // if it's a dynamic configuration option
if (DebugSessionOptions.isDynamic(option)) {
// Removing an item already present in the list
const index = this.recentDynamicOptionsTracker.findIndex(item => this.dynamicOptionsMatch(item, option));
if (index > -1) {
Expand All @@ -212,32 +212,33 @@ export class DebugConfigurationManager {
}
}

protected dynamicOptionsMatch(one: DebugSessionOptions, other: DebugSessionOptions): boolean {
protected dynamicOptionsMatch(one: DynamicDebugConfigurationSessionOptions, other: DynamicDebugConfigurationSessionOptions): boolean {
return one.providerType !== undefined
&& one.configuration.name === other.configuration.name
&& one.providerType === other.providerType;
&& one.configuration.name === other.configuration.name
&& one.providerType === other.providerType;
}

get recentDynamicOptions(): readonly DebugSessionOptions[] {
get recentDynamicOptions(): readonly DynamicDebugConfigurationSessionOptions[] {
return this.recentDynamicOptionsTracker;
}

protected updateCurrent(options: DebugSessionOptions | undefined = this._currentOptions): void {
this._currentOptions = options && this.find(options.configuration, options.workspaceFolderUri, options.providerType);
if (DebugSessionOptions.isCompound(options)) {
this._currentOptions = options && this.find(options.compound, options.workspaceFolderUri);
} else {
this._currentOptions = options && this.find(options.configuration, options.workspaceFolderUri, options.providerType);
}

if (!this._currentOptions) {
const model = this.getModel();
if (model) {
const configuration = model.configurations[0];
if (configuration) {
this._currentOptions = {
configuration,
workspaceFolderUri: model.workspaceFolderUri
};
this._currentOptions = this.configurationToOptions(configuration, model.workspaceFolderUri);
}
}
}
this.debugConfigurationTypeKey.set(this.current && this.current.configuration.type);
this.debugConfigurationTypeKey.set(this.current && this.current.configuration?.type);
this.onDidChangeEmitter.fire(undefined);
}

Expand All @@ -248,24 +249,57 @@ export class DebugConfigurationManager {
/**
* Find / Resolve DebugSessionOptions from a given target debug configuration
*/
find(targetConfiguration: DebugConfiguration, workspaceFolderUri?: string, providerType?: string): DebugSessionOptions | undefined;
find(nameOrTargetConfiguration: string | DebugConfiguration, workspaceFolderUri?: string, providerType?: string): DebugSessionOptions | undefined {
// providerType is only applicable to dynamic debug configurations
if (typeof nameOrTargetConfiguration === 'object' && providerType) {
return {
configuration: nameOrTargetConfiguration,
providerType
};
}
const name = typeof nameOrTargetConfiguration === 'string' ? nameOrTargetConfiguration : nameOrTargetConfiguration.name;
find(compound: DebugCompound, workspaceFolderUri?: string): DebugSessionOptions | undefined;
find(configuration: DebugConfiguration, workspaceFolderUri?: string, providerType?: string): DebugSessionOptions | undefined;
find(name: string, workspaceFolderUri?: string, providerType?: string): DebugSessionOptions | undefined;
find(nameOrConfigurationOrCompound: string | DebugConfiguration | DebugCompound, workspaceFolderUri?: string, providerType?: string): DebugSessionOptions | undefined {
if (DebugConfiguration.is(nameOrConfigurationOrCompound) && providerType) {
// providerType is only applicable to dynamic debug configurations and may only be created if we have a configuration given
return this.configurationToOptions(nameOrConfigurationOrCompound, workspaceFolderUri, providerType);
}
const name = typeof nameOrConfigurationOrCompound === 'string' ? nameOrConfigurationOrCompound : nameOrConfigurationOrCompound.name;
const configuration = this.findConfiguration(name, workspaceFolderUri);
if (configuration) {
return this.configurationToOptions(configuration, workspaceFolderUri);
}
const compound = this.findCompound(name, workspaceFolderUri);
if (compound) {
return this.compoundToOptions(compound, workspaceFolderUri);
}
}

findConfigurations(name: string, workspaceFolderUri?: string): DebugConfiguration[] {
const matches = [];
for (const model of this.models.values()) {
if (model.workspaceFolderUri === workspaceFolderUri) {
for (const configuration of model.configurations) {
if (configuration.name === name) {
return {
configuration,
workspaceFolderUri
};
matches.push(configuration);
}
}
}
}
return matches;
}

findConfiguration(name: string, workspaceFolderUri?: string): DebugConfiguration | undefined {
for (const model of this.models.values()) {
if (model.workspaceFolderUri === workspaceFolderUri) {
for (const configuration of model.configurations) {
if (configuration.name === name) {
return configuration;
}
}
}
}
}

findCompound(name: string, workspaceFolderUri?: string): DebugCompound | undefined {
for (const model of this.models.values()) {
if (model.workspaceFolderUri === workspaceFolderUri) {
for (const compound of model.compounds) {
if (compound.name === name) {
return compound;
}
}
}
Expand All @@ -279,6 +313,14 @@ export class DebugConfigurationManager {
}
}

protected configurationToOptions(configuration: DebugConfiguration, workspaceFolderUri?: string, providerType?: string): DebugSessionOptions {
return { name: configuration.name, configuration, providerType, workspaceFolderUri };
}

protected compoundToOptions(compound: DebugCompound, workspaceFolderUri?: string): DebugSessionOptions {
return { name: compound.name, compound, workspaceFolderUri };
}

async addConfiguration(): Promise<void> {
let rootUri: URI | undefined = undefined;
if (this.workspaceService.saved && this.workspaceService.tryGetRoots().length > 1) {
Expand Down Expand Up @@ -345,7 +387,8 @@ export class DebugConfigurationManager {
});
}
const root = await this.quickPickService.show(items, {
placeholder: nls.localize('theia/debug/addConfigurationPlaceholder', 'Select workspace root to add configuration to')
placeholder: nls.localize('theia/debug/addConfigurationPlaceholder', 'Select workspace root to add configuration to'),
ignoreFocusOut: true
});
return root?.value;
}
Expand Down Expand Up @@ -469,17 +512,26 @@ export class DebugConfigurationManager {

// Between versions v1.26 and v1.27, the expected format of the data changed so that old stored data
// may not contain the configuration key.
if (data.current && 'configuration' in data.current) {
if (DebugSessionOptions.isConfiguration(data.current)) {
// ensure options name is reflected from old configurations data
data.current.name = data.current.name ?? data.current.configuration?.name;
this.current = this.find(data.current.configuration, data.current.workspaceFolderUri, data.current.providerType);
} else if (DebugSessionOptions.isCompound(data.current)) {
this.current = this.find(data.current.name, data.current.workspaceFolderUri);
}
}

protected resolveRecentDynamicOptionsFromData(options?: DebugSessionOptions[]): void {
protected resolveRecentDynamicOptionsFromData(options?: DynamicDebugConfigurationSessionOptions[]): void {
if (!options || this.recentDynamicOptionsTracker.length !== 0) {
return;
}

this.recentDynamicOptionsTracker = options;
// ensure options name is reflected from old configurations data
const dynamicOptions = options.map(option => {
option.name = option.name ?? option.configuration.name;
return option;
}).filter(DebugSessionOptions.isDynamic);
Copy link
Contributor

Choose a reason for hiding this comment

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

is it really necessary to filter for isDynamic ?
non dynamic configs should not be available under the received argument:
options?: DynamicDebugConfigurationSessionOptions[], right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are absolutely correct that it should not be necessary. I added this guard because you had the case where there was a regular configuration in the 'recentDynamicOptions'. I could not reproduce the case but during runtime we already have a type guard before updating recently used dynamic options so I reasoned that it probably was caused during loading some old data. Since the recent options are limited to 3, I thought the extra safety outweighs any potential performance cost. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am leaning towards removing it, as the case may have been caused while switching among different versions of this change.
If we ever see it again, it's not too bad for the user to ignore it or replace it by making a new selection, and then we could try to fix the actual source of the problem.

this.recentDynamicOptionsTracker = dynamicOptions;
}

save(): void {
Expand All @@ -502,6 +554,6 @@ export class DebugConfigurationManager {
export namespace DebugConfigurationManager {
export interface Data {
current?: DebugSessionOptions,
recentDynamicOptions?: DebugSessionOptions[]
recentDynamicOptions?: DynamicDebugConfigurationSessionOptions[]
}
}
29 changes: 19 additions & 10 deletions packages/debug/src/browser/debug-configuration-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { Emitter, Event } from '@theia/core/lib/common/event';
import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable';
import { DebugConfiguration } from '../common/debug-common';
import { PreferenceService } from '@theia/core/lib/browser/preferences/preference-service';
import { DebugCompound } from '../common/debug-compound';

export class DebugConfigurationModel implements Disposable {

Expand Down Expand Up @@ -58,6 +59,10 @@ export class DebugConfigurationModel implements Disposable {
return this.json.configurations;
}

get compounds(): DebugCompound[] {
return this.json.compounds;
}

async reconcile(): Promise<void> {
this.json = this.parseConfigurations();
this.onDidChangeEmitter.fire(undefined);
Expand All @@ -66,25 +71,29 @@ export class DebugConfigurationModel implements Disposable {
const configurations: DebugConfiguration[] = [];
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const { configUri, value } = this.preferences.resolve<any>('launch', undefined, this.workspaceFolderUri);
if (value && typeof value === 'object' && 'configurations' in value) {
if (Array.isArray(value.configurations)) {
for (const configuration of value.configurations) {
if (DebugConfiguration.is(configuration)) {
configurations.push(configuration);
}
if (value && typeof value === 'object' && Array.isArray(value.configurations)) {
for (const configuration of value.configurations) {
if (DebugConfiguration.is(configuration)) {
configurations.push(configuration);
}
}
}
const compounds: DebugCompound[] = [];
if (value && typeof value === 'object' && Array.isArray(value.compounds)) {
for (const compound of value.compounds) {
if (DebugCompound.is(compound)) {
compounds.push(compound);
}
}
}
return {
uri: configUri,
configurations
};
return { uri: configUri, configurations, compounds };
}

}
export namespace DebugConfigurationModel {
export interface JsonContent {
uri?: URI
configurations: DebugConfiguration[]
compounds: DebugCompound[]
}
}
Loading