Support for Emmet preferences and syntaxProfiles

[mrmlnc]

Excuse

Sorry, but this is my first experience with the TypeScript. If everything is very bad, then let me know about this or add the proposed PR manually with correct code.

Why?

In the previous PR was added support Emmet for Sass and Stylus (#7887). That's cool, but Stylus has free syntax. For example:

.test {
  font-size: 14px;
  position absolute;
  display: block
}

By default, Emmet has the syntax: position absolute or font-size 14px. Using the Emmet preferences you can fix this. This PR adds support for Emmet preferences and syntaxProfiles.

Demo

• Emmet preferences:

  

• Emmet syntaxProfiles

  

Why you do not use emmet.preferences.load() or emmet.loadPreferences()

Unfortunately, both methods work only with emmet.preferences.set() and not provide access to emmet.preferences.define() that there is a need for rules that are not declared by default.

Question for developers

I'm not sure that the code in the constructor is correct. I couldn't think of a better way for access to Emmet one time.

    require(['emmet'], (_module) => {
        this.backupDefaultPreferences(_module);

        const disposable = configurationService.onDidUpdateConfiguration(e => this.updateEmmetConfig(_module, e.config));
        this.disposables.push(disposable);
    });

[msftclas]

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!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

[egamma]

@mrmlnc Thanks for the well documented Pull Request.

I've looked into the emmet preferences API and I don't fully understand your explanation of why you did not use emmet.loadPreferences(). It takes a JSON object with the preferences that could be taken from the VS Code configuration. emmet.loadPreferences calles preferences.load which calls preferences.set which sets the value of the preference.

If emmet.loadPreferences has limitations why do you not just iterate over the VS Code emmet configuraton settings and then call emmet.preferences.set(key, value) for each setting?

[egamma]

Now I see, set throws an exception, when the setting is not defined...

            Object.keys(prefs).forEach(function(k) {
                var v = prefs[k];
                if (!(k in defaults)) {
                    throw new Error('Property "' + k + '" is not defined. You should define it first with `define` method of current module');
                }

[egamma]

OK, I now catch the exception when the preferences isn't defined yet and then I define the setting.

I also use the approach to merge in the editor settings before an emmet operation and then I reset them after. This simplifies the implementation. Please review.

[mrmlnc]

@egamma, unfortunately, we are both wrong.

My version

Method emmet.loadPreferences() only works with the values described in the documentation (full list click). Defining settings (emmet.preferences.define()) need only for undocumented settings (for example, less.propertyEnd). For described values in the documentation enough use emmet.preferences.set(). Thus, can only use the method emmet.loadPreferences() for load preferences and emmet.preferences.reset() for reset preferences.

Your version

More likely, most users will not use the Emmet preferences and syntaxProfiles. Thus, the setting and resetting of settings when each action is not the best idea (deoptimization?).

For example:

My proposal

Use the methods emmet.preferences.reset() and emmet.loadPreferences() once during initialization and when changing settings. PR or demo repository? 👷

P.S.: Sorry for my English.

[egamma]

@mrmlnc

More likely, most users will not use the Emmet preferences and syntaxProfiles. Thus, the setting and resetting of settings when each action is not the best idea (deoptimization?).

I made some small changes so that there is no overhead when the user doesn't have emmet preferences. I must say I like the simplicity of the current code compared to the original PR. However, if the simple code is not correct, then it needs to be fixed 😄.

I'll also implement a solution that uses your approach and then we can compare. OK?

[mrmlnc]

@egamma, good idea.

I must say I like the simplicity of the current code compared to the original PR. However, if the simple code is not correct, then it needs to be fixed 😄.

The thing is that i originally thought to allow users to change all settings. But looking at the code of Emmet i realized that they are meaningless.

My vision settings for Emmet: mrmlnc/vscode/../emmetActions.ts

[nicothin]

@mrmlnc please tell me how to execute a command "wrap abbreviation" on hotkey?

[mrmlnc]

@nicothin,

1. VS Code insiders
2. File - Preferences - Keyboard Shortcuts
3. Press Ctrl + K, Ctrl + K (Windows)
4. Press desired key combination and Enter (for example: ctrl+alt+w)
5. In command section need to write emmet and press Ctrl + Space or Copy&Paste following code:

[{
    "key": "ctrl+alt+w",
    "command": "editor.emmet.action.wrapWithAbbreviation",
    "when": "editorTextFocus"
}]

[mrmlnc]

@egamma 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.

Disclaimer

I don't know how much is a correct test, but the results are interesting.

Tests

• Reload VS Code by F5 from http://localhost:9222/
• Run Emmet by Tab key

Results

Current version (first run ~300ms)

GIF

Current version witn Emmet bundle by Gulp (first run ~300ms)

GIF

My version (code) (first run ~150ms)

GIF

My version with Emmet bundle by Gulp (first run ~150ms)

GIF

[mrmlnc]

@egamma i just moved require(['emmet']... from promise run() into constructor. May need to check Emmet before it run: if (!this._emmet) { require(['emmet']... }.

https://gist.github.com/mrmlnc/3345b7d2a709e125d517e91ad1b9ca6f#file-emmet-ts-L42-L44

And settings apply when they are changed.

https://gist.github.com/mrmlnc/3345b7d2a709e125d517e91ad1b9ca6f#file-emmet-ts-L39-L40

[egamma]

@mrmlnc

i just moved require(['emmet']... from promise run() into constructor.

The EmmetAction constructor is called during startup. This means your version activates emmet on startup, which is what we want to avoid we do not want to add to the startup time. This is why we load emmet on the first tab key.

And settings apply when they are changed.

Keep in mind that the Action constructor is called for each action, this means you register a listener for each action (which I think there are now 17). This also means you update the preferences multiple times.

1. Moving the loading of emmet to the constructor is something we must not do.
2. The listener should only be registered once.

[mrmlnc]

@egamma, you are right.

[egamma]

@mrmlnc thanks for the investigation anyways, it shows that the bundling of emmet is not a win.