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

When wrapping with Emmet, consider the document language when setting the syntax. #68326

Merged
merged 4 commits into from
Feb 13, 2019

Conversation

mattkwiecien
Copy link
Contributor

@mattkwiecien mattkwiecien commented Feb 9, 2019

doWrapping was previously setting syntax = 'html when wrapping with abbreviations. This was resulting in bad class attribute names for jsx files.

  • Added a call to an existing function that determines the syntax based on the args parameter passed to doWrapping.
  • It's possible for this args parameter to be undefined, so added a guard clause to ensure that it is
    • not undefined, and
    • the language property is set by the current editor.document.languageId.

Now the syntax value is based on the return of getSyntaxFromArgs, and if the result of this function is undefined, default to 'html'.

This fixes #67098.

@ramya-rao-a
Copy link
Contributor

@mattkwiecien Nice! Do you mind adding a test case for this in wrapWithAbbreviation.test.ts file

@mattkwiecien
Copy link
Contributor Author

Absolutely! Will do either tonight or tomorrow morning.

@msftclas
Copy link

msftclas commented Feb 10, 2019

CLA assistant check
All CLA requirements met.

@mattkwiecien
Copy link
Contributor Author

Added two unit tests for wrapWithAbbreviation and wrapIndividualLinesWithAbbreviation in wrapWithAbbreviation.test.ts for when the file extension is 'jsx'.

While I was in there, I noticed that a lot of the unit tests were re-writing existing logic in the testWrapWithAbbreviation
function (and the newly added testWrapIndividualLinesWithAbbreviation function), so I refactored those tests to call that function instead of copying the same logic within each unit test.

@mattkwiecien
Copy link
Contributor Author

mattkwiecien commented Feb 11, 2019

@octref / @ramya-rao-a
Any more changes needed to make on this PR?

@octref
Copy link
Contributor

octref commented Feb 12, 2019

Just a small nitpick, looks good to me. Thanks!

@octref
Copy link
Contributor

octref commented Feb 13, 2019

Awesome contribution, thank you!

@octref octref merged commit 98f3ec8 into microsoft:master Feb 13, 2019
@octref octref added this to the February 2019 milestone Feb 13, 2019
@mattkwiecien
Copy link
Contributor Author

Thanks, really looking forward to contributing more to this great project :)

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

Emmet: Wrap with abbreviation in jsx not producing className
4 participants