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.

Sign up for GitHub

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

Jump to bottom

Emmet + selection gives weird behavior #53182

Closed
roblourens opened this issue Jun 27, 2018 · 8 comments
Closed

Emmet + selection gives weird behavior #53182

roblourens opened this issue Jun 27, 2018 · 8 comments
Assignees
@JacksonKearl
Labels
bug Issue identified by VS Code Team member as probable bug emmet Emmet related issues verified Verification succeeded
Milestone
July 2018

Comments

@roblourens
Copy link
Member

From #51180

I don't know if this is expected or not

<html>
	<body>
		<div />
	</body>
</html>
  • Select <div
  • Run "Split/Join Tag"
  • It actually operates on and produces <body />

And this seems related:

  • Start with the same html and selection
  • Run "Remove Tag"
  • body is deleted, you are left with <div /> which is weird, it deleted the opposite of the selection
  • "undo"
  • Now body has an extra close tag
<html>
	<body></body>
		<div />
	</body>
</html>
@ramya-rao-a ramya-rao-a added emmet Emmet related issues bug Issue identified by VS Code Team member as probable bug labels Jun 28, 2018
@ramya-rao-a ramya-rao-a added this to the July 2018 milestone Jun 28, 2018
@ramya-rao-a
Copy link
Contributor

@JacksonKearl Passing true for includeNodeBoundary in getHtmlNode will fix this. But before doing that, lets review all the usages of getHtmlNode and check where else inlcuding the boundary makes sense

@JacksonKearl
Copy link
Contributor

includeNodeBoundary is currently an optional parameter that defaults to false. It is left as that defualt value in:

  • updateTag
  • splitJoinTag
  • selectPrev/Next item
  • removeTag
  • balance (out only)

Of these, it is only important that selectPrev/Next item stays false and balance out stays false. The others could go either way.

@JacksonKearl JacksonKearl mentioned this issue Jun 28, 2018
Merged
@JacksonKearl
Copy link
Contributor

I put up a PR that explicitly passes includeNodeBoundries: false for selectPrev/Next item and balance out. We can now flip the default param to true if we decide that's better and everything should still work

@FerozCerebro13
Copy link

Flipped back to normal standards.

@JacksonKearl
Copy link
Contributor

As for the issue with tag auto-completion I believe we don't have direct access to the fact that the edit operation was an undo inside the extension, but we might be able to decide not to insert the closing tag if the edit operation inserted a closing tag itself? Relevant lines: https://github.com/Microsoft/vscode/blob/56fb08bd529544ac1eb9d89abd7a4af53bf508de/extensions/html-language-features/client/src/tagClosing.ts#L47

@ramya-rao-a
Copy link
Contributor

The issue of remove tag followed by undo was tracked in #39545 and the root cause was tracked in #34484

@ramya-rao-a
Copy link
Contributor

I put up a PR that explicitly passes includeNodeBoundries: false for selectPrev/Next item and balance out. We can now flip the default param to true if we decide that's better and everything should still work

@JacksonKearl Can you do a similar analysis to the use of getNode which has the same boundary related input parameter?

@JacksonKearl
Copy link
Contributor

@ramya-rao-a Put up a new commit that explicitly sets the getNode parameter where required (only one place I could find) I also flipped the default to true, as that seems like it makes more sense given emmet commands like "select prev/next item" put the selection at the boundaries of the node

@RMacfarlane RMacfarlane added the verified Verification succeeded label Aug 1, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@JacksonKearl JacksonKearl

Labels
bug Issue identified by VS Code Team member as probable bug emmet Emmet related issues verified Verification succeeded
Projects
None yet
Milestone
July 2018
Development

No branches or pull requests
5 participants
@roblourens @RMacfarlane @JacksonKearl @ramya-rao-a @FerozCerebro13