Add support for 'U' uppercase

[rebornix]

Convert selected range to Uppercase is an often-used command.

The code is well designed so it's easy enough to add new commands support. The only catch is all commands are implemented inside a single file, which makes it lack of categorization and readability, any ideas to refactor it?

[johnfn]

Hey @rebornix, thanks for the awesome PR! I'm glad to hear that the code is easy to understand for the most part.

I hear you about the gigantic file. Honestly, for whatever reason, I've always had a lower sensitivity to this issue than other people, which is why I haven't split it yet. When I tried briefly, I was having difficulty with decorators and splitting actions.ts into multiple files - I think TypeScript decorators may have some implementation bugs (they are classified as experimental, after all).

TL;DR - I'm totally open to PRs that split it up. It's a task I plan to do at some point, but it's not high priority. If a couple people nag me to do it, I'll bump the priority 😉

[johnfn]

Two comments here:

• adjustVimSelectionPosition is an ambiguous name. Something like transformRangeForDeleteOperator might be better.
• Instead of any I would prefer using a TypeScript tuple type: [Position, Position]

[johnfn]

Also I generally prefer static functions to free-floating ones. When we split this file up into many files, the decision will make our lives easier. 😃

[rebornix]

Thanks for the naming suggestion, it's always pain for non-English speaker like me :) The strict return type suggestion is also good, will change it later.

[johnfn]

Last thing - would you mind adding a few tests?

[rebornix]

@johnfn thanks for your detailed review and suggestion. I tried to run test cases on my Windows Machine previously but seems VS Code package will download Linux related package instead. I'll look into it and add tests 😃

[rebornix]

I've tried to move the start/end swapping for Visual mode but failed. The reason is that this will breaks deletion.

1. Delete Operator may be triggered by executeOperator or runAction, while we don't have any position information inside runAction, so we have to keep the swapping inside Delete Operator.
2. With above info in mind, we have the swapping inside Delete Op. So putting the swapping inside executeOperator will lead to the duplication of swapping for Delete Op as executeOperator might trigger Delete Op as well.

So the temporary solution for UpperCase operator is reusing the swapping method. This is caused by my limited knowledge of Action/Operator relationships and the workflow, once I have better understanding of the code, I may come up with a prettier solution.

[johnfn]

Hmm, I see. I will dig into this tomorrow and see what I can figure out!