The Wayback Machine - https://web.archive.org/web/20210307151408/https://github.com/microsoft/vscode/issues/110901
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

Emmet Expand Abbreviation should support expanding selection only #110901

Open
rzhao271 opened this issue Nov 19, 2020 · 9 comments
Open

Emmet Expand Abbreviation should support expanding selection only #110901

rzhao271 opened this issue Nov 19, 2020 · 9 comments

Comments

@rzhao271
Copy link
Contributor

@rzhao271 rzhao271 commented Nov 19, 2020

Version: 1.52.0-insider (user setup)
Commit: 78908e7
Date: 2020-11-18T12:38:39.842Z
Electron: 9.3.3
Chrome: 83.0.4103.122
Node.js: 12.14.1
V8: 8.3.110.13-electron.0
OS: Windows_NT x64 10.0.19042

Steps to Reproduce:

  1. Have emmet.triggerExpansionOnTab set to true
  2. Open an HTML file in VS Code, type testdiv+span
  3. Select div+span and explicitly run the command Emmet: Expand Abbreviation
  4. The abbreviation doesn't expand, instead, the selection is cleared and a tab is input instead

Does this issue occur when all extensions are disabled?: N/A

Also verified that this issue occurs on stable.
If the user merely presses tab, then the behaviour is understandable.
The confusing part is that the user is explicitly running the command, and a tab is inserted.

@rzhao271 rzhao271 added this to the Backlog milestone Nov 19, 2020
@rzhao271 rzhao271 self-assigned this Nov 19, 2020
@t7yang
Copy link

@t7yang t7yang commented Nov 26, 2020

I found another issue, when click [tab] on snippet tabstop wouldn't move to next tabstop instead trigger emmet expand (if emmet.triggerExpansionOnTab on).

Is this same problem?

@rzhao271
Copy link
Contributor Author

@rzhao271 rzhao271 commented Jan 6, 2021

@t7yang does that issue still occur for you on the latest Insiders? I'm unable to reproduce it. If it does, can you paste an example abbreviation for me to use?

@t7yang
Copy link

@t7yang t7yang commented Jan 6, 2021

@rzhao271 No, fixed now.

@rzhao271 rzhao271 changed the title Emmet Expand Abbreviation corner case with emmet.triggerExpansionOnTab true Emmet Expand Abbreviation should support expanding selection only Feb 26, 2021
@rzhao271 rzhao271 added feature-request and removed bug labels Feb 26, 2021
@abhiajju
Copy link
Contributor

@abhiajju abhiajju commented Mar 5, 2021

@rzhao271 I am unable to repro this issue. I tried to do an explicit Emmet: expand abbreviation and it worked fine for me. Can you please confirm ?
I did the exact steps as u have mentioned in the bug description

@abhiajju
Copy link
Contributor

@abhiajju abhiajju commented Mar 5, 2021

Oops!! I missed the first step in the repro - Have emmet.triggerExpansionOnTab set to true.
I can repro it now and what I also oberved is when this setting is false( which is by default) and if we explicitly run the command Emmet: Expand Abbreviation, the expansion works fine.
Looking into the issue

@abhiajju
Copy link
Contributor

@abhiajju abhiajju commented Mar 5, 2021

I found this piece of code in abbreviationAction.ts, inside the function expandEmmetAbbreviation() -

// When tabbed on a non empty selection, do not treat it as an emmet abbreviation, and fallback to tab instead
	if (vscode.workspace.getConfiguration('emmet')['triggerExpansionOnTab'] === true && editor.selections.find(x => !x.isEmpty)) {
		return fallbackTab();
	}

Seems like when the setting triggerExpansionOnTab is turned On, and we have a non-empty selection, we fallback to default Tab behavior

@rzhao271 can you please help me understand why we have this check in place, as I am uanble to comprehend any scenario that needs this.
I also verified that if we remove this code piece, we get the right behavior

@rzhao271
Copy link
Contributor Author

@rzhao271 rzhao271 commented Mar 5, 2021

Thanks for the update!
It seems Ramya added in that exact part of the code over 3 years ago but didn't link it to an issue: 78809b7
I'll change this issue to a Backlog Candidate feature. That way, we can see whether the community wants to enable Emmet tab expansions on selections.

@rzhao271 rzhao271 modified the milestones: Backlog, Backlog Candidates Mar 5, 2021
@rzhao271 rzhao271 removed the hackathon label Mar 5, 2021
@vscode-triage-bot
Copy link
Collaborator

@vscode-triage-bot vscode-triage-bot commented Mar 5, 2021

This feature request is now a candidate for our backlog. The community has 60 days to upvote the issue. If it receives 20 upvotes we will move it to our backlog. If not, we will close it. To learn more about how we handle feature requests, please see our documentation.

Happy Coding!

@abhiajju
Copy link
Contributor

@abhiajju abhiajju commented Mar 5, 2021

Thanks @rzhao271 . My only concern here is that for cases where we have emmet.triggerExpansionOnTab === false (which is by default), we can still expand the selection by explicitly running the command Emmet: Expand Abbreviation.
So , I don't see much utility of the above mentioned check.
Please let me know if I am missing something here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment