Add Command: Keyboard assignable DebugOutput Copy All

[cleidigh]

Add complementary keyboard command to copy all debug output.
Use new base from recently added Context Menu option

[isidorn]

@cleidigh thanks for creating a follow up issue!
After thinking about it I agree with your original proposal of creating a general command.

So what needs to be done is to register this action globally in the command palette somewhere around here

Let me know if you want to do a PR. If not, I can look into this.

[cleidigh]

@isidorn

Excellent. Very happy to do a PR ! ;-) (I have to learn to insert emojis)
I have one open PR waiting for review and I'm working on issue #25965, should be pushing something on that in a day or so. I can work on this while the others are spinning.

@Tyriar - does this overlap/replace the need for terminal.CopyAll ?

• if so have you started on it?

@isidorn - From your original comments/thoughts , should Problems output be excluded?
Name possibilities: (no current " .panel " name space
workbench.action.currentPanelViewCopyAll
workbench.panel.currentCopyAll
workbench.panelCopyAll
workbench.panelOutputCopyAll

I'm not sure what I like.
Do you both agree focus is not required?

Thanks

[Tyriar]

@cleidigh the terminal's getting a select all command when the work for #9958 completes.

@isidorn wouldn't focus/select commands be better than copying?

[cleidigh]

part of my main impetus was keyboard/steps conservation
my general idea of the use case:

• person is debugging or following output while in the editor view
• wants a snapshot - one keyboard action - (manually followed by) paste into current editor/new editor - no navigation needed

[isidorn]

@Tyriar select all command would be better than copying however we are not supporting this. You can read more about it here

@cleidigh as I already mentioned in the other issue it is hard to have one action which will do it for all the panels. Let's just do a PR to add a Debug: Copy All From Debug Console same as there is already Debug: Focus into Console and Debug: Clear Console

p.s on the mac there is a nice feature I press cmd + ctrl + space and I get a nice emoji picker 😁

[cleidigh]

@isidorn
@Tyriar
Sounds good, I'll take that route.
(have to get a Chrome plug-in for Windows ;-)

[Tyriar]

@isidorn

In order to make copy work we would need to append selected textas we are scrolling in the tree.

I don't think your assertion is correct here, you want to listen to the copy event and then set the text based on what the selection is, similar to this https://github.com/sourcelair/xterm.js/blob/35b32b721e9be175c2eedb83c264442d171152b5/src/handlers/Clipboard.ts#L59

[isidorn]

@Tyriar yes but the tree is virtualised so the native selection can never span more than a page of text in the tree. For more details you can read up on that whole issue

[Tyriar]

@isidorn ah yeah, a copy all command is definitely the easy way to go. If you're interested how I'm solving that problem in the terminal you can check out https://github.com/Tyriar/xterm.js/tree/207_selection_manager, it's probably easier to do within the VS Code codebase though thanks to ScrollableElement.

[cleidigh]

@isidorn
@Tyriar
I got a chance to start looking at the code today. it seems to copy all action has a different signature
for the constructor that wants the internal tree perimeter. as far as I can tell it is not possible
to register the command in this form. I'm looking for similar examples perhaps an overloaded constructor?
I'll still have to deal with scope access to the tree and not break any layering.
this is probably something fairly basic I'm missing.

[cleidigh]

looks like maybe adding to DebugActions/repl services to expose a get repl tree contents and then
redo the copy all action to have a signature2 style and then register that command replacing the old style.

[isidorn]

@cleidigh you are right this is something which I missed. The constructore signiture makes this a bit more complicated and would break the layering.

Due to that I propose we introduce a top level debug panel action next to the Clear Console. This way keyboard users need to press shift + tab 3 times to get to this action. They of course need to be in "Tab moves focus" mode. For this we would just need an icon but I can come up with on.
Let me know if you think this is not a good solution and I can think of how we can fix the layers to support the top level command palette action.

There is no need to rush this in. Pushing to June.

[cleidigh]

@isidorn
I think the copy all icon makes sense. I still kind of lean to a global command as well, but not a
super high priority as you said. I think I see a couple of ways to expose the action
with clean layering. I will try to come up with a possible solution and run it by you here.

[isidorn]

@cleidigh sounds good, thanks

[isidorn]

@cleidigh let me know if there are some updates here, if you did not find a neat solution for the global command I might add it to the title bar.

[cleidigh]

@isidorn
I've looked at the code quite a bit, the repl object seems buried in the panel part.
I believe only one object is instantiated, but I don't see an accounting mechanism.
adding an ID to the create call, would allow for subsequent queries to gain access
to the tree object. that could then be exposed either through the panel interface,
or perhaps the debug actions service.
I think the above would still be necessary for adding anything to the title bar as well

can you think of another way to expose the object without breaking layering?

[isidorn]

@cleidigh yeah the issue is that the tree is not globaly accessible, and it can be passed only in the local actions. Or we could surface the tree which feels quite ugly to me

[cleidigh]

@isidorn
the other output sources are accessible via ID. why should the depot console not somehow be integrated this way. I saw another issue post talking about a copy all. it still seems like this make sense
to expose some how. is the repl object meant to be singular? if so then exposing the tree contents
would not be a big issue.

[cleidigh]

@isidorn
I tried one of my earlier ideas. extending the privateReplService. that actually worked
it doesn't totally make sense considering it is called without an object in the accessor get call
I will post the code tomorrow, maybe you can tell me why no object is needed ;-)

[cleidigh]

@isidorn
Added copy all to the private service interface
added an editor command
use the private service accessor that gives access to the tree object (I cannot figure out why this does not need an object reference)
duplicated the copy all code from the context menu

everything is done in repl.ts

export interface IPrivateReplService {
_serviceBrand: any;
navigateHistory(previous: boolean): void;
acceptReplInput(): void;
copyRepl(): void;
}

@editorAction
class ReplCopyAllAction extends EditorAction {

constructor() {
	super({
		id: 'repl.action.copyall',
		label: nls.localize('actions.repl.copyall', "Copy All"),
		alias: 'Copy All',
		precondition: null ,
		kbOpts: {
			kbExpr: EditorContextKeys.textFocus,
			primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.KEY_A,
			weight: 50
		}

	});
}

public run(accessor: ServicesAccessor, editor: ICommonCodeEditor): void | TPromise<void> {
	accessor.get(IPrivateReplService).copyRepl();
}

}

public copyRepl(): void {
	console.log('CopyAll Repl');
	let text = '';
	const navigator = this.tree.getNavigator();
	// skip first navigator element - the root node
	while (navigator.next()) {
		if (text) {
			text += `\n`;
		}
		text += navigator.current().toString();
	}
	clipboard.writeText(text);
}

[isidorn]

@cleidigh I like this solution a lot! Can you create a nice PR please and ping me on it?
Thanks!

[cleidigh]

@isidorn
Cool
I will clean it up and do a PR
I will set it up with no preassigned key
-global context as well

[Tyriar]

@cleidigh FYI there is a new command workbench.action.terminal.selectAll in the latest Insiders which selects all text. Bound by default on macOS to cmd+a but not Windows/Linux (as ctrl+a is a command shell shortcut).