'Copy all' for context menu in output view

[salkin-naj]

What it does

This adds another option to the context menu of the output view.
It copies everything in the selected channel into the clipboard.

How to test

1. Open output view
2. Select a channel with text in it
3. Open context menu and select 'Copy All'

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[vince-fugnitto]

@salkin-naj we have a clipboard-service which should be used instead and has handling for specific cases:

theia/packages/core/src/browser/clipboard-service.ts

Lines 20 to 23 in d017cb2

	export interface ClipboardService {
	readText(): MaybePromise<string>;
	writeText(value: string): MaybePromise<void>;
	}

[salkin-naj]

@vince-fugnitto: I tried using the clipboardService in the outputContribution, but I get circular dependencies that I cannot figure out. Is there something I need to do? In preferenceContribution it is used exactly like I want to use it and it works fine.

[vince-fugnitto]

@vince-fugnitto: I tried using the clipboardService in the outputContribution, but I get circular dependencies that I cannot figure out. Is there something I need to do? In preferenceContribution it is used exactly like I want to use it and it works fine.

@salkin-naj you can push your changes so far and I can take a look at it with you to help you along the way if you'd like :)
There should not be any circular dependencies present since the service is from core, at a high-level it should look something along the lines of:

import { ClipboardService } from '@theia/core/lib/browser/clipboard-service';

export class OutputContribution extends AbstractViewContribution<OutputWidget> implements OpenHandler {

@inject(ClipboardService)
protected readonly clipboardService: ClipboardService;

...

this.clipboardService.writeText('foo');
}

[vince-fugnitto]

The changes work well with the service, I have some comments about some improvements.

[vince-fugnitto]

Rename to Copy All

[vince-fugnitto]

@salkin-naj we should only register the keybinding when the view has focus else it will override all other commands with ctrlc+shift+c which is not the behavior we would want or expect.

We can do so by implementing a when context similarly to how it is done in other views:

• https://github.com/eclipse-theia/theia/blob/master/packages/scm/src/browser/scm-context-key-service.ts#L20-L46

Please let me know if you require help, I could assist you :)

[kittaakos]

Everybody can do Ctrl/⌘+A then Ctrl/⌘+C. We do not need a keybinding for this. Please remove.

[kittaakos]

It looks great, @salkin-naj. Let's simplify a little bit and were are ready to go ;)

Thank you! 🙏

[kittaakos]

Let's return undefined when the model value cannot be retrieved.

    getText(): string | undefined {
        const editor = this.editor;
        if (editor) {
            const model = editor.getControl().getModel();
            if (model) {
                return model.getValue();
            }
        }
        return undefined;
    }

[kittaakos]

Thank you for your contribution, @salkin-naj 🥇

[kittaakos]

Please make sure you have squashed and the commit is signed.

[salkin-naj]

Please make sure you have squashed and the commit is signed.

@kittaakos Done :)

[kittaakos]

@salkin-naj, I found a bug. I cannot accept the PR as is 😕

I propose the following:

diff --git a/packages/output/src/browser/output-contribution.ts b/packages/output/src/browser/output-contribution.ts
index daf4a4903..b16646b75 100644
--- a/packages/output/src/browser/output-contribution.ts
+++ b/packages/output/src/browser/output-contribution.ts
@@ -102,9 +102,7 @@ export namespace OutputCommands {
     };
 
     export const COPY_ALL: Command = {
-        id: 'output:copy-all',
-        label: 'Copy All',
-        category: OUTPUT_CATEGORY,
+        id: 'output:copy-all'
     };
 }
 
@@ -161,7 +159,8 @@ export class OutputContribution extends AbstractViewContribution<OutputWidget> i
             commandId: CommonCommands.COPY.id
         });
         registry.registerMenuAction(OutputContextMenu.TEXT_EDIT_GROUP, {
-            commandId: OutputCommands.COPY_ALL.id
+            commandId: OutputCommands.COPY_ALL.id,
+            label: 'Copy All'
         });
         registry.registerMenuAction(OutputContextMenu.COMMAND_GROUP, {
             commandId: quickCommand.id,

If you add a label to the command, it will show up in the Command Palette (F1 or Ctrl/⌘+Shift+P) if it's enabled. Since the handler for the command does not define the isEnabled/isVisible function, the command will be always enabled and visible. So if I close the Output view and open the Command Palette and search for Output: Copy All, it is visible and I can execute it. Yes, it does not do anything but it should not be there.

👆Solution: move the label property from the command and move it to the menu.

[kittaakos]

Thanks for your persistence on this PR, @salkin-naj