TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

move to code actions update and tests

Open justschen opened this issue 1 year ago • 2 comments

Fixes #https://github.com/microsoft/TypeScript/issues/58387

as discussed in the sync, we added the additional check to implicit and also mirrored the fixes from move to file for move to new file.

a lot of tests were added (if excessive we can delete), and fixes to move to new file were added but also understand that move to new file will likely be deprecated anyways eventually.


regarding changing: https://github.com/microsoft/TypeScript/blob/5c21b7fd93cc60b05d560f931fb8bb26d7972f76/tests/cases/fourslash/moveToNewFile_prologueDirectives4.ts#L8

since getApplicableRefactors defaults to implicit: https://github.com/microsoft/TypeScript/blob/5c21b7fd93cc60b05d560f931fb8bb26d7972f76/src/harness/fourslashImpl.ts#L4454

the test was changed to have no refactorings (to match our proposed case).


regarding implicit vs invoked:

Check for implicit to follow our idea for manual requested in vs code vs. not manually requested in vs code, but let me know if our logic here makes sense!

cc. @aiday-mar @mjbvz

justschen avatar May 16 '24 07:05 justschen

update @aiday-mar, looks like triggerReason gets passed all the way from typescript extension:

https://github.com/microsoft/vscode/blob/cdf94536b9dafc36582d2ecfd246897c6feb7d72/extensions/typescript-language-features/src/languageFeatures/refactor.ts#L551

private toTsTriggerReason(context: vscode.CodeActionContext): Proto.RefactorTriggerReason | undefined {
	return context.triggerKind === vscode.CodeActionTriggerKind.Invoke ? 'invoked' : 'implicit';
}

which gets propagated thru CodeActionTriggerKind between automatic and invoke which we set in the model 👍

justschen avatar May 16 '24 17:05 justschen

Looks good, thanks!

aiday-mar avatar May 17 '24 07:05 aiday-mar

cc. @andrewbranch @navya9singh @DanielRosenwasser (realized i never pinged or mentioned you guys on this 😨)

justschen avatar May 21 '24 19:05 justschen

@andrewbranch @navya9singh awesome thanks guys! feel free to merge when ready (I'm not authorized here 🔢 )

justschen avatar May 23 '24 19:05 justschen