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] Support all the features #8155

Merged
merged 12 commits into from
Jun 27, 2016
Merged

[emmet] Support all the features #8155

merged 12 commits into from
Jun 27, 2016

Conversation

mrmlnc
Copy link
Contributor

@mrmlnc mrmlnc commented Jun 26, 2016

Adding support for all features of Emmet.

Source: #7909

Description

Well, Emmet. Sorry, that one commit.

What include?

All the features of Emmet:

What's not included?

I am not binding keyboard keys as it can cause a conflict.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @egamma and @alexandrudima to be potential reviewers

@msftclas
Copy link

Hi @mrmlnc, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@egamma egamma self-assigned this Jun 26, 2016
@egamma
Copy link
Member

egamma commented Jun 26, 2016

I'll start to digest the PR tomorrow, first scan shows that this is good work 🌹

One convention you missed is this one:
https://github.com/Microsoft/vscode/wiki/Coding-Guidelines#strings
We have lint checks based on this convention that finds strings that are not localizable.

@mrmlnc
Copy link
Contributor Author

mrmlnc commented Jun 26, 2016

@egamma, oh, sorry, i forgot about it. Now fix it 👍

@egamma
Copy link
Member

egamma commented Jun 26, 2016

@mrmlnc this is a question unrelated to the PR, but given your emmet know how I thought I'll ask you. Do you know the purpose of the gulpfile inside the emmet module? It looks like it supports to package/bundle emmet. Is any other editor using this? I ask since the startup time of emmet is significant and we look into options to reduce it.

@egamma
Copy link
Member

egamma commented Jun 26, 2016

There is some duplication in the code that I introduced and that should be improved, eventually.

This method can be written generically when the action is configured with the emmet action name.

    public runEmmetAction(_module) {
        if (!_module.run('balance_outward', this.editorAccessor)) {
            this.editorAccessor.noExpansionOccurred();
        }
    }

Something like

class BasicEmmetEditorAction extends EmmetEditorAction {
    ...
        private emmetActionName: string;

    constructor(descriptor: IEditorActionDescriptorData, editor: ICommonCodeEditor, actionName: string) {
        super(descriptor, editor, Behaviour.TextFocus);
        this.editorAccessor = new EditorAccessor(editor);
                this.emmetActionName = actionName;
    }

    public runEmmetAction(_module) {
        if (!_module.run(emmetActionName, this.editorAccessor)) {
            this.editorAccessor.noExpansionOccurred();
        }
    }   

I'll first start with the PR and I'll refactor and you can review.

@mrmlnc
Copy link
Contributor Author

mrmlnc commented Jun 27, 2016

@egamma Do you know the purpose of the gulpfile inside the emmet module?

Just build all files into one with Browserify. Simply launch the Gulp and get the output files. For example:

conemu64_2016-06-27_05-34-56

And these files can be used in VS Code:

photoshop_2016-06-27_05-13-36

Can play with the settings Browserify that to get a size smaller. For example (fullPaths: false):

conemu64_2016-06-27_05-35-47

Or even more, using bundle-collapser plugin:

conemu64_2016-06-27_05-36-20

@egamma egamma mentioned this pull request Jun 27, 2016
@egamma egamma merged commit 2a15286 into microsoft:master Jun 27, 2016
egamma added a commit that referenced this pull request Jun 27, 2016
@egamma egamma mentioned this pull request Jun 27, 2016
17 tasks
@egamma
Copy link
Member

egamma commented Jun 27, 2016

@mrmlnc I almost forgot to say thank you 🌹. I was busy to get the PR into for the June update before we have the code freeze and start the endgame.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
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.

4 participants