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

Support multiple selections (multi-cursor) when inserting converted time #49

Open
healarconr opened this issue Mar 27, 2021 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@healarconr
Copy link

Is your feature request related to a problem? Please describe.

I'm always frustrated when I want to insert multiple converted times because I have to run a conversion command for every time that I want to convert. When I run a conversion command having multiple selections the converted time is inserted only for the first selection.

Describe the solution you'd like

I would like that the conversion commands would insert all the converted times when the editor has multiple selections instead of just the first one. It would be similar to the way commands like "Transform to Uppercase" or "Transform to Lowercase" work.

Additional context

multiple-selections

@HaaLeo HaaLeo added enhancement New feature or request help wanted Extra attention is needed labels Mar 27, 2021
@HaaLeo
Copy link
Owner

HaaLeo commented Mar 27, 2021

Hi @healarconr, thx for raising this issue. I think we should definitly aim for multi cursor support. I would happily accept a PR for this issue. If somebody wants to take over this issue I would offer my support and guidance to get started.
Otherwise, I'll implement it myself when I find some spare time. However, I do not know when that will be the case 😅.

@healarconr
Copy link
Author

I saw that this method returns the text of the first selection:

private getSelection(): string | undefined {
let result: string;
const activeEditor = vscode.window.activeTextEditor;
if (activeEditor !== undefined) {
const activeSelection = activeEditor.selection;
if (!activeSelection.isEmpty) {
result = activeEditor.document.getText(activeSelection);
}
}
return result;
}

Maybe it can filter empty selections and return Selection[].

This other method uses it but also reads from the clipboard in case there is no selected text:

protected async getPreInput(): Promise<string> {
let result = this.getSelection();
if (!result && this._readInputFromClipboard) {
result = await vscode.env.clipboard.readText();
}
return result;
}

Maybe a class that represents the text and an optional selection is needed there.

Then the commands seem to pass the input text through some steps. Maybe these commands can iterate over the inputs obtained from getPreInput().

Finally, the conversion result is inserted:

protected insert(insertion: string): Thenable<boolean> {
const activeEditor = vscode.window.activeTextEditor;
if (activeEditor !== undefined) {
return activeEditor.edit(editBuilder => {
editBuilder.replace(activeEditor.selection, insertion);
});
}
}

It could use the optional Selection of every input to replace the corresponding time.

@HaaLeo
Copy link
Owner

HaaLeo commented Mar 28, 2021

I think the "conversion steps" must not be altered. I think there are quite some (edge) cases we must consider. For example what should happen if multiple rows are selected, but result insertion is turned off. Is the first conversion result shown? What if the multi selection consists of different timestamp formats. For example not only epoch timestamps. Should this lead to an error?

I think this is a quite challenging issue with lots of different cases to be considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants