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

Change Language IDs to comply with guidelines (Closes #86) #87

Merged
merged 1 commit into from
May 2, 2020

Conversation

zolrath
Copy link

@zolrath zolrath commented Apr 30, 2020

@axelson Here you are! I'm not sure of the full extent of testing this that should occur or if we should combine its merger with pull requests on other popular packages supporting html.eex to reduce the impact of the change.

The language identifier guidelines state that language ids should be lowercase and contain no spaces.
https://code.visualstudio.com/docs/languages/identifiers#_new-identifier-guidelines

This mostly is a port of changes that were never merged for the old vscode-elixir definition.
timmhirsens/vscode-elixir#114

The largest impact of this change is that other extensions that rely on the current identifier would have to be changed for support to continue.

Fixes #86

Copy link
Member

@axelson axelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! ❤️ This looks like a good change to bring the extension into alignment with VSCode guidelines. Hopefully the migration won't be too difficult for users. I'll hold off on merging until we write a short guide on adjusting configuration to match, if it is short enough it can live in ElixirLS's changelog, if it is longer it can be a new .md file in this repo.

Here's some related issues that we may additionally want to drop a comment on:

Related issues:

A PR should also be filed against:

@zolrath
Copy link
Author

zolrath commented May 1, 2020

vscode-tailwindcss now has support for html-eex
https://github.com/bradlc/vscode-tailwindcss/pull/111/files

and I've submitted a pull request for vscode-eex-snippets
stefanjarina/vscode-eex-snippets#5

@zolrath
Copy link
Author

zolrath commented May 1, 2020

As far as things to put in the changelog/document things that people may have copy/pasted into their settings and forgotten about that they should remember to update would include:
If you're using Emmet for HTML expansion be sure to include the new Language Id in your settings.json file.

Ctrl+Shift+P or Cmd+Shift+P and type Preference: Open Settings (JSON)
Add or edit your emmet.includedLanguages to include the new Language Id:

"emmet.includeLanguages": {
    "html-eex": "html"
}

@axelson
Copy link
Member

axelson commented May 2, 2020

Thank you very much for the upgrade guide and for making those PR's! I'm going to go ahead and merge this now 🎉

@stefanjarina
Copy link

Hello all, just FYI, pull request for this merged to stefanjarina/vscode-eex-snippets.
Also new version 0.0.7 already available in marketplace.
Thank you a lot for PR :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Eex Language IDs compliant with guidelines?
3 participants