vscode-timing icon indicating copy to clipboard operation
vscode-timing copied to clipboard

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

Open healarconr opened this issue 4 years ago • 3 comments

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

healarconr avatar Mar 27 '21 03:03 healarconr

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 😅.

HaaLeo avatar Mar 27 '21 13:03 HaaLeo

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

https://github.com/HaaLeo/vscode-timing/blob/b3f6bb71a792b1bede0f1016243d12f37e4ac250/src/commands/commandBase.ts#L128-L139

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:

https://github.com/HaaLeo/vscode-timing/blob/b3f6bb71a792b1bede0f1016243d12f37e4ac250/src/commands/commandBase.ts#L58-L66

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:

https://github.com/HaaLeo/vscode-timing/blob/b3f6bb71a792b1bede0f1016243d12f37e4ac250/src/commands/commandBase.ts#L68-L75

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

healarconr avatar Mar 28 '21 03:03 healarconr

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.

HaaLeo avatar Mar 28 '21 16:03 HaaLeo