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

Jump to bottom

Create a new 'Open in Trace Viewer' command, add it to navigator context menu #1119

Merged
merged 2 commits into from
Sep 25, 2024
Merged

Create a new 'Open in Trace Viewer' command, add it to navigator context menu #1119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1eabed7
Removed usage of Theia platform 'open-with' service
marcdumais-work Sep 19, 2024
85775c9
Create a new 'Open in Trace Viewer' command, add it to navigator cont…
marcdumais-work Sep 19, 2024
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
6 changes: 3 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,9 @@ This is the most intuitive way to open traces and trace groups, but it can only

#### Via the File Explorer

You can open any supported trace format via the file explorer context menu. For a single trace, right-click on the trace file, or folder (for a CTF trace), then select **Open With → Open Trace**. To open several CTF trace files as a group, right-click on the parent folder instead.
You can open any supported trace format via the file explorer context menu. For a single trace, right-click on the trace file, or folder (for a CTF trace), then select **Open in Trace Viewer**. To open several CTF trace files as a group, right-click on the parent folder instead.

![Open With Trace Viewer][image-open-with]
![Open in Trace Viewer][image-open-in-trace-viewer]

### Open a view

Expand Down Expand Up @@ -466,7 +466,7 @@ The code in this repository is licensed under `MIT` (see root `LICENSE`), except
[image-icon]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-icon.png
[image-open-browser]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-browser.png
[image-open-view]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-view.gif
[image-open-with]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-with-trace-viewer.gif
[image-open-in-trace-viewer]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-in-trace-viewer.gif
[image-properties]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-item-properties-0.0.2.png
[image-sidebar]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-open-trace-viewer.gif
[image-viewer]: https://raw.githubusercontent.com/eclipse-cdt-cloud/theia-trace-extension/master/doc/images/theia-trace-extension-0.0.3.png
Expand Down
Binary file added doc/images/theia-trace-extension-open-in-trace-viewer.gif
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file removed doc/images/theia-trace-extension-open-with-trace-viewer.gif
View file
Open in desktop
Binary file not shown.
5 changes: 5 additions & 0 deletions theia-extensions/viewer-prototype/src/browser/trace-explorer/trace-explorer-commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,9 @@ export namespace TraceExplorerCommands {
id: 'trace-explorer:remove-trace',
label: 'Remove Trace'
};

export const OPEN_IN_TRACE_VIEWER: Command = {
id: 'trace-explorer:open-in-trace-viewer',
label: 'Open in Trace Viewer'
marcdumais-work marked this conversation as resolved.
Show resolved Hide resolved
};
Copy link
Collaborator

@bhufmann bhufmann Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure where to add the comment. Could you please change the priority of the menu entry of that command so that it's lower in the list, preferably last. We did the same change in the vscode-trace-extension PR 249 after receiving user feedback that it's too intrusive (i.e. too high up with the common open commands). See image. Not sure how to do that in Theia but it should be possible.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change the priority of the menu entry of that command so that it's lower in the list, preferably last

easily done

[use "open in trace viewer"] we should also change it in the vscode-trace-extension

Created an issue, that we can schedule as we see fit: eclipse-cdt-cloud/vscode-trace-extension#275

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a new version that should address the comments. The context menu entry should be the last item in the "Navigation" section (top part) of the menu. We can't control the order of new entries that might get added by others, but with the vanilla entries, added by Theia, we should be consistently at the end of the section.

Copy link
Contributor Author

@marcdumais-work marcdumais-work Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

One question I have: are you bugged by the constant and "id" using "OPEN_WITH..." / "...open-with..." but presenting the command as "open in..."?:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If so, I can align everything to use "open in"

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's now or never because after we release it the command could be triggered by a third-party extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - let's do it then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, new version pushed

}
23 changes: 22 additions & 1 deletion theia-extensions/viewer-prototype/src/browser/trace-explorer/trace-explorer-contribution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
import { injectable } from '@theia/core/shared/inversify';
import { injectable, inject } from '@theia/core/shared/inversify';
import { AbstractViewContribution } from '@theia/core/lib/browser/shell/view-contribution';
import { TraceExplorerWidget } from './trace-explorer-widget';
import { FrontendApplicationContribution, FrontendApplication } from '@theia/core/lib/browser';
import { MenuModelRegistry, CommandRegistry } from '@theia/core';
import { TraceExplorerCommands, TraceExplorerMenus } from './trace-explorer-commands';
import { NavigatorContextMenu } from '@theia/navigator/lib/browser/navigator-contribution';
import { UriAwareCommandHandler } from '@theia/core/lib/common/uri-command-handler';
import { SelectionService } from '@theia/core/lib/common';
import { TraceViewerContribution } from '../trace-viewer/trace-viewer-contribution';

@injectable()
export class TraceExplorerContribution
extends AbstractViewContribution<TraceExplorerWidget>
implements FrontendApplicationContribution
{
@inject(SelectionService) protected readonly selectionService: SelectionService;
@inject(TraceViewerContribution) protected readonly traceViewer: TraceViewerContribution;

constructor() {
super({
widgetId: TraceExplorerWidget.ID,
Expand Down Expand Up @@ -44,6 +51,11 @@ export class TraceExplorerContribution
label: TraceExplorerCommands.REMOVE_TRACE.label,
order: 'c'
});

menus.registerMenuAction(NavigatorContextMenu.NAVIGATION, {
commandId: TraceExplorerCommands.OPEN_IN_TRACE_VIEWER.id,
order: 'z9'
});
}

async registerCommands(registry: CommandRegistry): Promise<void> {
Expand All @@ -67,5 +79,14 @@ export class TraceExplorerContribution
explorerWidget.deleteExperiment(traceUUID);
}
});

registry.registerCommand(
TraceExplorerCommands.OPEN_IN_TRACE_VIEWER,
UriAwareCommandHandler.MonoSelect(this.selectionService, {
execute: uri => {
this.traceViewer.open(uri);
}
})
);
}
}
17 changes: 2 additions & 15 deletions theia-extensions/viewer-prototype/src/browser/trace-viewer/trace-viewer-contribution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { injectable, inject, postConstruct } from '@theia/core/shared/inversify';
import { injectable, inject } from '@theia/core/shared/inversify';
import { CommandRegistry, CommandContribution, MessageService } from '@theia/core';
import {
WidgetOpenerOptions,
WidgetOpenHandler,
KeybindingContribution,
KeybindingRegistry,
OpenWithService
KeybindingRegistry
} from '@theia/core/lib/browser';
import URI from '@theia/core/lib/common/uri';
import { TraceViewerWidget, TraceViewerWidgetOptions } from './trace-viewer';
Expand Down Expand Up @@ -53,7 +52,6 @@ export class TraceViewerContribution
@inject(TracePreferences) protected tracePreferences: TracePreferences;
@inject(TraceServerConfigService) protected readonly traceServerConfigService: TraceServerConfigService;
@inject(MessageService) protected readonly messageService: MessageService;
@inject(OpenWithService) protected readonly openWithService: OpenWithService;
@inject(TraceServerConnectionStatusClient)
protected readonly serverStatusService: TraceServerConnectionStatusClient;

Expand All @@ -68,17 +66,6 @@ export class TraceViewerContribution
return this.tracePreferences[TRACE_ARGS];
}

@postConstruct()
protected init(): void {
this.openWithService.registerHandler({
id: this.id + '-open-with',
label: this.label,
providerName: 'Theia Trace Viewer',
canHandle: (uri: URI) => this.canHandle(uri),
open: (uri: URI) => this.open(uri)
});
}

protected createWidgetOptions(uri: URI, options?: TraceViewerWidgetOpenerOptions): TraceViewerWidgetOptions {
return {
traceURI: uri.path.toString(),
Expand Down
Loading