Add `v-bind` code action #2524

zardoy · 2023-03-19T15:56:52Z

Fixes #2087

Quotes are not added by design, as the code action intended for cases where user meant an identifier

So @jd-solanki could add the following keybinding:

{
    "key": "...",
    "command": "editor.action.codeAction",
    "args": {
        "kind": "refactor.rewrite.addVBind",
        "apply": "ifSingle"
    }
}

IMO its worth also adding : when : typed after attribute name, it would be faster for me rather than adding additional keybinding.

johnsoncodehk · 2023-03-19T18:57:55Z

Thanks for the PR! Are you planning to change the code action to two-way conversion?

IMO its worth also adding : when : typed after attribute name, it would be faster for me rather than adding additional keybinding.

This is the implementation, but really not sure we should have it.

provideAutoInsertionEdit(document, position, insertCtx) {

	if (insertCtx.lastChange.text !== ':') return;

	const offset = document.offsetAt(position);
	const [virtualFile, source] = ctx!.documents.getVirtualFileByUri(document.uri);
	if (!virtualFile || !(source.root instanceof VueFile)) {
		return;
	}

	const { templateAst } = source.root.sfc;

	if (!templateAst) return;

	let result: vscode.TextEdit | undefined;

	walkElementNodes(templateAst, node => {
		for (const prop of node.props) {
			if (
				prop.type === NodeTypes.ATTRIBUTE
				&& offset === prop.loc.start.offset + prop.name.length
			) {
				result = {
					newText: ':' + prop.name.slice(0, -1) + '$0',
					range: {
						start: document.positionAt(prop.loc.start.offset),
						end: document.positionAt(prop.loc.start.offset + prop.name.length),
					},
				};
			}
		}
	});

	return result;
}

zardoy · 2023-03-19T22:25:00Z

Are you planning to change the code action to two-way conversion?

I was thinking of adding it, but only value is string: :type="'password'" -> type="password", but it would require to walk every node used within v-bind attribute and I currently don't know how to do it (tried but failed in #2496)

I was also thinking of adding ="$0" for current code action when attribute have no value.

This is the implementation, but really not sure we should have it.

If you're against of having this feature, I don't think I need implementation, but thanks. Though I don't see any downsides of having it e.g. typing : after attribute name currently don't have correct usages, right? Anyway, its just a proposal: we don't think it will be useful - let's don't add it here.

packages/vue-language-service/src/plugins/vue-toggle-v-bind-codeaction.ts

zardoy · 2023-03-19T22:40:58Z

How its possible code action to use snippet edit? Want to move cursor inside added empty value.

rchl · 2023-03-19T22:47:36Z

How its possible code action to use snippet edit? Want to move cursor inside added empty value.

You can't. microsoft/language-server-protocol#592

BTW. This probably would be more useful if it would be automatically provided in relevant contexts (whether cursor is within the attribute's key or value. Then it wouldn't require custom key bindings and could be used by any LSP client by just triggering the default key binding for showing code actions.

zardoy · 2023-03-19T23:09:59Z

This probably would be more useful if it would be automatically provided in relevant contexts (whether cursor is within the attribute's key or value. Then it wouldn't require custom key bindings and could be used by any LSP client by just triggering the default key binding for showing code actions.

Sorry, didn't get what you mean. It is already using regular code action provider that doesn't require any additional keybindings in vscode and I guess any lsp client can use it?

Also

whether cursor is within the attribute's key or value

Doesn't the current impl already do it?

rchl · 2023-03-19T23:14:04Z

Doesn't the current impl already do it?

Hmm, right. I guess it should. But I can't get this plugins' provideCodeActions implementation to trigger in my testing.

I'm testing on a Vue 2.7 project, if that matters.

johnsoncodehk · 2023-03-19T23:17:36Z

How its possible code action to use snippet edit? Want to move cursor inside added empty value.

I think you need to use CodeAction.command to insert =, and then language servers will trigger doQuoteComplete() to insert "" and make the cursor in it.

packages/vue-language-service/src/languageService.ts

rchl · 2023-03-19T23:25:52Z

The issue with not seeing this code action is due to some bigger issue with Volar itself. Between 1.2.1 and 1.3.0 it just stopped functioning properly and now it stalls after sending diagnostics...

zardoy · 2023-03-20T00:12:55Z

How its possible code action to use snippet edit? Want to move cursor inside added empty value.

I think you need to use CodeAction.command to insert =, and then language servers will trigger doQuoteComplete() to insert "" and make the cursor in it.

Done, but it would really better to use SnippetEdit in future as now it requires to undo twice to get content before code action

packages/vue-language-service/src/plugins/vue-toggle-v-bind-codeaction.ts

jd-solanki · 2023-03-20T05:19:09Z

Thanks @zardoy for the implementation

Does this mean, we can now add v-bind or : (shorthand) to existing attribute via hotkey?

That's super helpful 🚀

rchl · 2023-03-20T07:34:03Z

Tried to test it a little:

Screen.Recording.2023-03-20.at.08.31.50.mov

:: [08:30:11.320] <<< LSP-volar (13) (duration: 7ms): [{'edit': {'changes': {'file:///usr/local/workspace/temp/nuxt-2-async-data/pages/indexWithObjectProps.vue': [{'range': {'end': {'character': 20, 'line': 2}, 'start': {'character': 12, 'line': 2}}, 'newText': ''}]}}, 'title': 'Add v-bind to attribute', 'kind': 'refactor.rewrite.addVBind', 'data': {'original': {'edit': {'changes': {'file:///usr/local/workspace/temp/nuxt-2-async-data/pages/indexWithObjectProps.vue': [{'range': {'end': {'character': 20, 'line': 2}, 'start': {'character': 12, 'line': 2}}, 'newText': ''}]}}}, 'pluginId': 'vue/toggleVBindCodeaction', 'uri': 'file:///usr/local/workspace/temp/nuxt-2-async-data/pages/indexWithObjectProps.vue', 'type': 'plugin', 'version': 1}, 'command': {'title': '', 'arguments': [{'text': ':disabled='}], 'command': 'type'}}]
:: [08:30:13.423] --> LSP-volar workspace/executeCommand (14): {'arguments': [{'text': ':disabled='}], 'command': 'type'}
:: [08:30:13.424] <~~ LSP-volar (14) (duration: 1ms): {'message': 'Unhandled method workspace/executeCommand', 'code': -32601}

Is this change tying its functionality to the vscode extension (or even vscode itself)? Because it doesn't seem like the server supports type workspace command.

Kinda weird to split the response into a text edit and a command when everything can be pretty easily done with just a single text edit.

EDIT: I see it's due to this comment:

I think you need to use CodeAction.command to insert =, and then language servers will trigger doQuoteComplete() to insert "" and make the cursor in it.

But I guess you meant "editors" and not "language servers" and more like "VSCode" rather than "editors"

zardoy · 2023-03-20T13:22:26Z

But I guess you meant "editors" and not "language servers" and more like "VSCode" rather than "editors"

You're absolutely right, but I didn't find a way to figure out which client made request (e.g. vscode) from ctx.capabilities, so I can use workaround for vscode only.

@johnsoncodehk I think it'd be better to provide ctx.commands.createSnippetEditCommand for vscode, which would register custom command to do editor.applySnippet(..., range) but it requires changes in other repos... Or I can just revert ac6e3fa

rchl · 2023-03-20T13:27:12Z

You shouldn't be making workarounds for specific editors and I hope @johnsoncodehk thinks the same.

If you must make something vscode-specific then do it in the vscode extension. You can probably even intercept commands and add whatever you want there.

Think of creating a behavior that is best for all LSP-complient clients.

I wouldn't consider it a breaking issue if the user has to move the cursor manually to the attribute value after triggering this code action.

This reverts commit ac6e3fa.

zardoy · 2023-03-20T13:41:49Z

If you must make something vscode-specific then do it in the vscode extension.

Isn't ctx.commands are already vscode-specific?

I wouldn't consider it a breaking issue if the user has to move the cursor manually to the attribute value after triggering this code action.

Agree, it's not breaking, but it could be a huge ux improvement, though I see a logical limitation in code actions support, so I reverted that commit with the workaround.

vuejs/language-tools#2524

johnsoncodehk · 2023-03-21T21:21:47Z

How its possible code action to use snippet edit? Want to move cursor inside added empty value.

I use setSelection VSCode command to support it. For other IDEs the cursor just has no move, I think it's fine.

We can merge the PR when you implement "Toggle".

zardoy · 2023-03-22T02:18:28Z

We can merge the PR when you implement "Toggle".

Do you mean Remove v-bind refactoring for this case?

<Test :prop="'string'" /> -> <Text prop="string" />

will do!

zardoy · 2023-03-24T01:21:05Z

Hey @johnsoncodehk, I was thinking of the following detection for this code action:

find node at position of v-bind directive
check this node is string and end == v-bind directive end

But I'm not sure how to obtain correct range of sourceFile. I tried to add this to for (const prop of node.props) {:

if (
	startOffset - templateStartOffset >= prop.loc.start.offset
	&& endOffset - templateStartOffset <= prop.loc.end.offset
	&& prop.type === NodeTypes.DIRECTIVE && prop.exp
) {
	for (const [_, map] of ctx!.documents.getMapsByVirtualFileUri(document.uri)) {
		const r = map.toGeneratedRange({
			start: document.positionAt(prop.exp.loc.start.offset),
			end: document.positionAt(prop.exp.loc.end.offset)
		});
		if (!r) continue;
		const ts = ctx!.typescript!.module;
		const sourceFile = ctx!.typescript!.languageService.getProgram()!.getSourceFile(_.mainScriptName)!;
		const startOffset = ts.getPositionOfLineAndCharacter(sourceFile, r.start.line, r.start.character);
		const endOffset = ts.getPositionOfLineAndCharacter(sourceFile, r.end.line, r.end.character);
		const generatedText = sourceFile.getFullText().slice(startOffset, endOffset);
		// or the same, but much simpler
		const generatedText = map.virtualFileDocument.getText(r)
	}
}

But this doesn't seem to work. Is there an easy way to get that range in virtual script file?

johnsoncodehk · 2023-03-25T05:04:15Z

@zardoy Sorry I probably didn't understand your request, is this what you're asking for?

if (
	startOffset - templateStartOffset >= prop.loc.start.offset
	&& endOffset - templateStartOffset <= prop.loc.end.offset
	&& prop.type === NodeTypes.DIRECTIVE && prop.exp
) {
	const nodeAtPosition = prop;
	const nodeIsString = (prop.exp.loc.source.startsWith('"') && prop.exp.loc.source.endsWith('"'))
		|| (prop.exp.loc.source.startsWith("'") && prop.exp.loc.source.endsWith("'"));
	const isEndOfDirective = endOffset - templateStartOffset === prop.arg?.loc.end.offset;
}

rchl · 2023-03-25T14:52:01Z

packages/vue-language-service/src/plugins/vue-toggle-v-bind-codeaction.ts

+								edit: {
+									changes: { [document.uri]: edits },
+								},
+								command: newPosition ? ctx?.commands.createSetSelectionCommand(newPosition) : undefined,


It's still using a custom command which will trigger a user facing error in form a dialog box because this is how ST handles invalid commands. I don't know about how other editors handle unsupported commands.

I will add an option to let the language client prompt for a list of supported commands.

zardoy · 2023-03-25T15:50:53Z

@johnsoncodehk This is too naive. I wanted to use TypeScript API to do this check: Let's say I have :foo="'a' + b + 'c'" your check will pass, but we could get range of virtual file instead ('a' + __VLS_ctx.b + 'c') and with TS API do this check:

check this node is string and end == v-bind directive end

johnsoncodehk · 2023-03-25T16:39:10Z

@zardoy Since :foo="'a' + b + 'c'" will create multiple mappings, there is only one verbose way to do it.

if (
	startOffset - templateStartOffset >= prop.loc.start.offset
	&& endOffset - templateStartOffset <= prop.loc.end.offset
	&& prop.type === NodeTypes.DIRECTIVE && prop.exp
) {
	for (const [tsFile, map] of ctx!.documents.getMapsByVirtualFileName(vueFile.mainScriptName)) {
		const tsStart = map.map.toGeneratedOffset(prop.exp.loc.start.offset + templateStartOffset, false)?.[0];
		const tsEnd = map.map.toGeneratedOffset(prop.exp.loc.end.offset + templateStartOffset, true)?.[0];
		const ast = ctx?.typescript?.languageService.getProgram()?.getSourceFile(tsFile.fileName);
		if (tsStart !== undefined && tsEnd !== undefined && ast) {
			let node: ts.Node | undefined;
			ast.forEachChild(function visit(_node) {
				if (_node.getStart(ast) === tsStart && _node.getEnd() === tsEnd) {
					node = _node;
				}
				if (!node && _node.getStart(ast) <= tsStart && _node.getEnd() >= tsEnd) {
					_node.forEachChild(visit);
				}
			});
			console.log(tsStart, tsEnd, node?.getText(ast));
		}
	}
}

You may also consider another simpler alternative.

const ast = ts.createSourceFile('/a.ts', prop.exp.loc.source, ts.ScriptTarget.Latest);

check this node is string and end == v-bind directive end

Did it mean endOffset - templateStartOffset === prop.loc.end.offset?

zardoy · 2023-03-25T17:05:00Z

const ast = ts.createSourceFile('/a.ts', prop.exp.loc.source, ts.ScriptTarget.Latest);

I believe creating new source file on every cursor change wihtin directive isn't cheap. However I see there is prop.exp.loc.__volar_ast that already contains this file, but I don't see typings for this. Can I reuse it? I would do this simple check then: prop.exp.loc.__volar_ast.statements.length === 1 && ts.isStringLiteralLike(prop.exp.loc.__volar_ast.statements[0]). Otherwise I'd try to use first solution (will try in other pr anyway).

johnsoncodehk · 2023-03-25T17:13:19Z

__volar_ast is internal and it can be unpredictable, e.g. it sometimes wraps statements in (), but yes you can try it. Also I think the cost of doing createSourceFile for a single ExpressionNode is almost negligible.

johnsoncodehk · 2023-07-21T16:49:42Z

Made some refinements, hope not messed up. Thanks!

zardoy and others added 6 commits March 19, 2023 18:36

Add v-bind code action

6d1ee1a

cleanup findTemplateNode

67c661c

remove uneeded check as they have different types

7781752

Merge branch 'master' into pr/2524

232520f

refactor: use walkElementNodes()

8ac5186

chore: remove log

de21c41

zardoy commented Mar 19, 2023

View reviewed changes

packages/vue-language-service/src/plugins/vue-toggle-v-bind-codeaction.ts Show resolved Hide resolved

add value when missing one, remove findTemplateNode

0027932

rchl reviewed Mar 19, 2023

View reviewed changes

packages/vue-language-service/src/languageService.ts Outdated Show resolved Hide resolved

johnsoncodehk and others added 3 commits March 20, 2023 07:35

perf: dont loop props if out of range

85225c0

use ??=

e126fac

use command for adding empty value content

ac6e3fa

nekocom3745 approved these changes Mar 20, 2023

View reviewed changes

johnsoncodehk reviewed Mar 20, 2023

View reviewed changes

packages/vue-language-service/src/plugins/vue-toggle-v-bind-codeaction.ts Outdated Show resolved Hide resolved

fix path point to out

7e0000c

Revert "use command for adding empty value content"

d6b7acf

This reverts commit ac6e3fa.

johnsoncodehk added a commit to volarjs/volar.js that referenced this pull request Mar 21, 2023

feat: support setSelection command

b0fc46b

vuejs/language-tools#2524

feat: use createSetSelectionCommand() [skip ci]

d2a87f6

Merge branch 'master' into pr/2524

7ca5ac6

johnsoncodehk mentioned this pull request Mar 25, 2023

Extract into simple component refactoring #2496

Merged

rchl reviewed Mar 25, 2023

View reviewed changes

add remove v-bind code action

421a9d9

zardoy requested review from johnsoncodehk and nekocom3745 and removed request for johnsoncodehk and nekocom3745 March 25, 2023 17:36

zardoy and others added 2 commits March 25, 2023 20:39

add comment

3db552a

Merge branch 'master' into pr/2524

f97805c

johnsoncodehk force-pushed the master branch from e653641 to 5c42710 Compare May 22, 2023 09:55

johnsoncodehk mentioned this pull request Jun 12, 2023

Unable to focus on Issues currently #3271

Closed

8 tasks

johnsoncodehk added 5 commits July 22, 2023 00:12

Merge branch 'master' into pr/2524

153340f

Update vue-toggle-v-bind-codeaction.ts

b0666c3

done

f16bc6d

cache ast

83d3ae7

undo ast cache

a27ec4c

johnsoncodehk merged commit 0301988 into vuejs:master Jul 21, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `v-bind` code action #2524

Add `v-bind` code action #2524

zardoy commented Mar 19, 2023

johnsoncodehk commented Mar 19, 2023

zardoy commented Mar 19, 2023

zardoy commented Mar 19, 2023

rchl commented Mar 19, 2023 •

edited

zardoy commented Mar 19, 2023

rchl commented Mar 19, 2023

johnsoncodehk commented Mar 19, 2023 •

edited

rchl commented Mar 19, 2023 •

edited

zardoy commented Mar 20, 2023 •

edited

jd-solanki commented Mar 20, 2023

rchl commented Mar 20, 2023 •

edited

zardoy commented Mar 20, 2023 •

edited

rchl commented Mar 20, 2023

zardoy commented Mar 20, 2023 •

edited

johnsoncodehk commented Mar 21, 2023

zardoy commented Mar 22, 2023

zardoy commented Mar 24, 2023

johnsoncodehk commented Mar 25, 2023

rchl Mar 25, 2023

johnsoncodehk Mar 25, 2023

zardoy commented Mar 25, 2023 •

edited

johnsoncodehk commented Mar 25, 2023 •

edited

zardoy commented Mar 25, 2023 •

edited

johnsoncodehk commented Mar 25, 2023

johnsoncodehk commented Jul 21, 2023

Add v-bind code action #2524

Add v-bind code action #2524

Conversation

zardoy commented Mar 19, 2023

johnsoncodehk commented Mar 19, 2023

zardoy commented Mar 19, 2023

zardoy commented Mar 19, 2023

rchl commented Mar 19, 2023 • edited

zardoy commented Mar 19, 2023

rchl commented Mar 19, 2023

johnsoncodehk commented Mar 19, 2023 • edited

rchl commented Mar 19, 2023 • edited

zardoy commented Mar 20, 2023 • edited

jd-solanki commented Mar 20, 2023

rchl commented Mar 20, 2023 • edited

zardoy commented Mar 20, 2023 • edited

rchl commented Mar 20, 2023

zardoy commented Mar 20, 2023 • edited

johnsoncodehk commented Mar 21, 2023

zardoy commented Mar 22, 2023

zardoy commented Mar 24, 2023

johnsoncodehk commented Mar 25, 2023

rchl Mar 25, 2023

Choose a reason for hiding this comment

johnsoncodehk Mar 25, 2023

Choose a reason for hiding this comment

zardoy commented Mar 25, 2023 • edited

johnsoncodehk commented Mar 25, 2023 • edited

zardoy commented Mar 25, 2023 • edited

johnsoncodehk commented Mar 25, 2023

johnsoncodehk commented Jul 21, 2023

Add `v-bind` code action #2524

Add `v-bind` code action #2524

rchl commented Mar 19, 2023 •

edited

johnsoncodehk commented Mar 19, 2023 •

edited

rchl commented Mar 19, 2023 •

edited

zardoy commented Mar 20, 2023 •

edited

rchl commented Mar 20, 2023 •

edited

zardoy commented Mar 20, 2023 •

edited

zardoy commented Mar 20, 2023 •

edited

zardoy commented Mar 25, 2023 •

edited

johnsoncodehk commented Mar 25, 2023 •

edited

zardoy commented Mar 25, 2023 •

edited