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

Consider moving emmet completion provider for css to css language service #29113

Closed
ramya-rao-a opened this issue Jun 20, 2017 · 17 comments
Closed
Assignees
Labels
debt Code quality issues emmet Emmet related issues
Milestone

Comments

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jun 20, 2017

In the new emmet, you get css abbreviations inside the <style> tag. But you get it everywhere inside the style tag including the selectors which is not valid.

Contents of <style> tag should be re-parsed as css to find valid locations for emmet abbr expansion

Or the emmet completion provider for css should be moved to the css language service

@ramya-rao-a ramya-rao-a added emmet Emmet related issues feature-request Request for new features or functionality labels Jun 20, 2017
@ramya-rao-a ramya-rao-a added this to the Backlog milestone Jun 20, 2017
@ramya-rao-a ramya-rao-a added the help wanted Issues identified as good community contribution opportunities label Jun 20, 2017
@ramya-rao-a ramya-rao-a modified the milestones: July 2017, Backlog Jun 22, 2017
@ramya-rao-a ramya-rao-a changed the title Reparse contents of <style> tag as css to enable emmet abbr validation Consider moving emmet completion provider for css to css language service Jun 26, 2017
@ramya-rao-a
Copy link
Contributor Author

ramya-rao-a commented Jun 26, 2017

Moving emmet completion to css language service will also help in sorting the emmet suggestion among css suggestions See the discussion in #28933

This will also help giving css emmet completions inside the style attribute in html tags. See #29066

@ramya-rao-a
Copy link
Contributor Author

The sorting issue has been fixed with microsoft/vscode-emmet-helper@04593ca

Abbreviations like ttu (emmet givestext-transform: uppercase) or tac (emmet gives text-align: center; will now be shown on the top

@ramya-rao-a
Copy link
Contributor Author

ramya-rao-a commented Dec 20, 2017

Things to do to get emmet support in css language service:

  • Get UMD version of the emmet helper and hook it up with the language service
  • Pass emmet settings from css client to css language service initially and on every change in these settings
  • Currently the syntax profiles, variables and custom snippets can be defined in an external file, that is read by the vscode-emmet-helper module and then combined with the values from the settings. This needs to move out to the css client and passed to the language service along with rest of the emmet settings

@ramya-rao-a
Copy link
Contributor Author

ramya-rao-a commented Jan 26, 2018

Update:
We ended up having the emmet logic in the css language server instead of the css language service
Most of the required work is done. See #41652
Waiting for next version of LSP that will have the change for new completion trigger kind, and so this is moved to the next iteration

@jens1o
Copy link
Contributor

jens1o commented Jan 28, 2018

In case you need somebody for extensive testing, I'm available for you! :)

@ramya-rao-a
Copy link
Contributor Author

Thanks @jens1o!
I'll mention you here in this issue when its ready which should be sometime in the second week of February.

@ramya-rao-a
Copy link
Contributor Author

Calling for emmet champions, especially the ones who helped me a ton last year
@smlombardi, @jens1o, @vvs, @niichie, @tklepzig

This feature is now complete and can be tested on latest Insiders dated 2/13
If you are using Win 64 bits, then please wait another day and test on the Insiders dated 2/14

Try out css emmet completions in css/less/scss files and also inside the style block in html files

@jens1o
Copy link
Contributor

jens1o commented Feb 13, 2018

Yeeaaaahh. But now I have to wait until tomorrow. :/

@smlombardi
Copy link

!important does not offer a completion

@ramya-rao-a
Copy link
Contributor Author

@smlombardi Fixed. Try again with tomorrow's build

@jens1o
Copy link
Contributor

jens1o commented Feb 18, 2018

Working great so far!

Thank you very much. :)

@ramya-rao-a ramya-rao-a added verification-needed Verification of issue is requested verified Verification succeeded labels Feb 27, 2018
@ramya-rao-a
Copy link
Contributor Author

This work has been reverted due to #44840 and #44352.
Re-opening and moving to March

@ramya-rao-a ramya-rao-a reopened this Mar 5, 2018
@ramya-rao-a ramya-rao-a modified the milestones: February 2018, March 2018 Mar 5, 2018
@ramya-rao-a ramya-rao-a added debt Code quality issues and removed help wanted Issues identified as good community contribution opportunities verification-needed Verification of issue is requested verified Verification succeeded feature-request Request for new features or functionality labels Mar 5, 2018
@jens1o
Copy link
Contributor

jens1o commented Mar 5, 2018

Will this affect the insiders release, too?

@ramya-rao-a
Copy link
Contributor Author

@jens1o Yes, currently its reverted in Insiders release and will not be present in the upcoming 1.21 stable release.

@jens1o
Copy link
Contributor

jens1o commented Mar 5, 2018

Alright, I'll keep my eye sharp to test it once it's back ;)

@ramya-rao-a
Copy link
Contributor Author

microsoft/vscode-css-languageservice#81 and microsoft/vscode-css-languageservice#69 are 2 issues in the css language service that stops us from moving emmet to the css extension

@ramya-rao-a ramya-rao-a modified the milestones: March 2018, Backlog Mar 26, 2018
@ramya-rao-a
Copy link
Contributor Author

Below were the advantages we were hoping for by moving the emmet css completions to the css extension

  • Emmet css completions in style attribute in html files
  • Reusing the parsing done by the css extension and not relying on the css parser from emmet which tends to get much slower for larger files.

The second problem got solved when we implemented the partial parsing of css files last milestone. See #43470 (comment)

The first problem can be solved in other ways.

Since we have upstream issues that block this move and there isn't much to gain out of this move, we are closing this item and will continue to keep css emmet completions in the emmet extension itself

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues emmet Emmet related issues
Projects
None yet
Development

No branches or pull requests

4 participants
@smlombardi @jens1o @ramya-rao-a and others