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

Add doNotAddAttributeQuotes setting to disable automatic quotes #129284

Merged
merged 3 commits into from Oct 15, 2021

Conversation

ssigwart
Copy link
Contributor

@ssigwart ssigwart commented Jul 24, 2021

This depends on microsoft/vscode-html-languageservice#112

This allows users to disable the automatic quotes after an HTML attribute. I'm not sure the best way to test this since it depends on the pull request in microsoft/vscode-html-languageservice. I tested by manually modifying extensions/html-language-features/server/node_modules/vscode-html-languageservice/lib/umd/services/htmlCompletion.js to apply the changes.

A simple test is to use following and trigger the class completion:

<div clas|

With this setting enabled, it should be class=|, not class="|".

@@ -188,6 +188,12 @@
"default": true,
"description": "%html.autoClosingTags%"
},
"html.doNotAddAttributeQuotes": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should follow the style of the existing settings, e.g. html.completion.attributeQuotes

@aeschli
Copy link
Contributor

aeschli commented Aug 17, 2021

I'd rather avoid new settings as they make the product more complex and add are costly to maintain.
So before adding a setting I'd like to see more user request for it.

@ssigwart
Copy link
Contributor Author

Thanks, @aeschli. Where would other users request it? Should I add an issue so it can go into the backlog candidate flow.

Also, do you think I'd be able to write an extension to accomplish this? I can't think of a way without writing a whole LSP extension for HTML, which would be overkill.

For reference, I personally prefer typing the quote myself. I can probably train myself to stop type the quote, but the bigger issue is that is seems like VSCode doesn't recognize trigger characters if they were part of a completion. That seems to be the issue in zignd/HTML-CSS-Class-Completion#235 and gencer/SCSS-Everywhere#36 for CSS completion. Currently, the way to work around that is to let class="|" auto-complete, then hit backspace, then type the quote and CSS completion will work. If the quotes weren't auto-inserted, it would be more intuitive.

Alternatively, would it be acceptable to write a patch where if you have editor.autoClosingOvertype set to never and type " in class="|" just after it's inserted, it will assume you meant to type the opening brace an overwrite it similar to the editor.autoClosingOvertype setting? Currently, it winds up as class=""|".

@aeschli
Copy link
Contributor

aeschli commented Aug 18, 2021

Yes, an issue that goes through backlog candidate would be the way to get other users feedback and votes.

I would be more open to it we could make setting a bit more general. Some users might prefer single quotes
html.completion.attributeDefaultValue=empty | singleQuotes | doubleQuotes

@ssigwart
Copy link
Contributor Author

Thanks, @aeschli. I like that idea. I updated this pull request and microsoft/vscode-html-languageservice#112 to do that. I also opened #131144 to track if there's interest in this.

@ssigwart
Copy link
Contributor Author

@aeschli, I updated this to match the changes in microsoft/vscode-html-languageservice#112. I rebased it and now there's an error with typescript, which I didn't touch, so I ignored it. Please let me know if I should rebased at a later point once that error is resolved.

@aeschli
Copy link
Contributor

aeschli commented Sep 30, 2021

That looks good, thanks @ssigwart ! I'll merge it next week.

@ssigwart
Copy link
Contributor Author

ssigwart commented Oct 1, 2021

Thanks, @aeschli. I'm really looking forward to this setting.

@aeschli aeschli closed this Oct 15, 2021
@aeschli aeschli reopened this Oct 15, 2021
@aeschli aeschli merged commit 8d70e04 into microsoft:main Oct 15, 2021
@aeschli
Copy link
Contributor

aeschli commented Oct 15, 2021

Thanks @ssigwart !

@ssigwart ssigwart deleted the noQuotes branch October 16, 2021 15:42
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants