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

Fixes vscode #65464 #46

Merged
merged 1 commit into from
Feb 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions src/emmetHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export function doComplete(document: TextDocument, position: Position, syntax: s
let expandedAbbr: CompletionItem;
let completionItems: CompletionItem[] = [];

// Create completion item after expanding given abbreviation
// Create completion item after expanding given abbreviation
// if abbreviation is valid and expanded value is not noise
const createExpandedAbbr = (syntax: string, abbr: string) => {
if (!isAbbreviationValid(syntax, abbreviation)) {
Expand Down Expand Up @@ -192,7 +192,7 @@ export function doComplete(document: TextDocument, position: Position, syntax: s
if (expandedAbbr && abbreviationSuggestions.length > 0 && tagToFindMoreSuggestionsFor !== abbreviation) {
expandedAbbr.sortText = '0' + expandedAbbr.label;
abbreviationSuggestions.forEach(item => {
// Workaround for snippet suggestions items getting filtered out as the complete abbr does not start with snippetKey
// Workaround for snippet suggestions items getting filtered out as the complete abbr does not start with snippetKey
item.filterText = abbreviation
// Workaround for the main expanded abbr not appearing before the snippet suggestions
item.sortText = '9' + abbreviation;
Expand All @@ -215,7 +215,7 @@ export function doComplete(document: TextDocument, position: Position, syntax: s
}

/**
* Create & return snippets for snippet keys that start with given prefix
* Create & return snippets for snippet keys that start with given prefix
*/
function makeSnippetSuggestion(
snippetKeys: string[],
Expand Down Expand Up @@ -455,7 +455,7 @@ export function extractAbbreviation(document: TextDocument, position: Position,
}

/**
* Extracts abbreviation from the given text
* Extracts abbreviation from the given text
* @param text Text from which abbreviation needs to be extracted
* @param syntax Syntax used to extract the abbreviation from the given text
*/
Expand Down Expand Up @@ -495,7 +495,11 @@ export function isAbbreviationValid(syntax: string, abbreviation: string): boole
return false;
}
if (abbreviation.includes('#')) {
return hexColorRegex.test(abbreviation) || propertyHexColorRegex.test(abbreviation);
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder whether propertyHexColorRegex should still show up. For example, bgc:#333 is valid (and expands to background-color: #333;).

Copy link
Contributor Author

@jeanp413 jeanp413 Feb 8, 2021

Choose a reason for hiding this comment

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

There are tests for that case and they still pass

const testCases: [string, string][] = [
['#1', '#111'],
['#ab', '#ababab'],
['#abc', '#abc'],
['c:#1', 'color: #111;'],
['c:#1a', 'color: #1a1a1a;'],
['bgc:1', 'background-color: 1px;'],
['c:#0.1', 'color: rgba(0, 0, 0, 0.1);']
];

if (abbreviation.startsWith('#')) {
return hexColorRegex.test(abbreviation);
} else if (commonlyUsedTags.includes(abbreviation.substring(0, abbreviation.indexOf('#')))) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what this case is for, considering we're in a if (isStyleSheet(syntax)) block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rzhao271 The whole if (abbreviation.includes('#')) block was added as a fix for this microsoft/vscode#56082 where emmet completions would show for id selectors and that's not desired. propertyHexColorRegex regex is too restrictive causing expressions like bd1#s not being expanded so the fix is to not show completions only for id selectors of the form <element>#<id>

}
}
return cssAbbreviationRegex.test(abbreviation);
}
Expand Down Expand Up @@ -575,7 +579,7 @@ export function getExpandOptions(syntax: string, emmetConfig?: VSCodeEmmetConfig
const formatters = getFormatters(syntax, emmetConfig['preferences']);
const unitAliases: SnippetsMap = (formatters?.stylesheet && formatters.stylesheet['unitAliases']) || {};

// These options are the default values provided by vscode for
// These options are the default values provided by vscode for
// extension preferences
const defaultVSCodeOptions: Partial<Options> = {
// inlineElements: string[],
Expand Down Expand Up @@ -767,7 +771,7 @@ function applyVendorPrefixes(expandedProperty: string, vendors: string, preferen

/**
* Parses given abbreviation using given options and returns a tree
* @param abbreviation string
* @param abbreviation string
* @param options options used by the emmet module to parse given abbreviation
*/
export function parseAbbreviation(abbreviation: string, options: UserConfig): StylesheetAbbreviation | MarkupAbbreviation {
Expand Down Expand Up @@ -805,7 +809,7 @@ export function expandAbbreviation(abbreviation: string | MarkupAbbreviation | S

/**
* Maps and returns syntaxProfiles of previous format to ones compatible with new emmet modules
* @param syntax
* @param syntax
*/
function getProfile(syntax: string, profilesFromSettings: object): any {
if (!profilesFromSettings) {
Expand Down Expand Up @@ -1031,7 +1035,7 @@ export async function updateExtensionsPath(emmetExtensionsPath: string | undefin
const profilesDataStr = new TextDecoder().decode(profilesData);
profilesFromFile = JSON.parse(profilesDataStr);
} catch (e) {
//
//
}
}

Expand All @@ -1047,10 +1051,10 @@ function resetSettingsFromFile() {
/**
* Get the corresponding emmet mode for given vscode language mode
* Eg: jsx for typescriptreact/javascriptreact or pug for jade
* If the language is not supported by emmet or has been exlcuded via `exlcudeLanguages` setting,
* If the language is not supported by emmet or has been exlcuded via `exlcudeLanguages` setting,
* then nothing is returned
*
* @param language
*
* @param language
* @param exlcudedLanguages Array of language ids that user has chosen to exlcude for emmet
*/
export function getEmmetMode(language: string, excludedLanguages: string[] = []): string | undefined {
Expand All @@ -1071,7 +1075,6 @@ export function getEmmetMode(language: string, excludedLanguages: string[] = [])
}
}

const propertyHexColorRegex = /^[a-zA-Z]+:?#[\d.a-fA-F]{0,6}$/;
const hexColorRegex = /^#[\d,a-f,A-F]{1,6}$/;
const onlyLetters = /^[a-z,A-Z]+$/;

Expand Down Expand Up @@ -1127,6 +1130,3 @@ export function getEmmetCompletionParticipants(document: TextDocument, position:
}
};
}



2 changes: 1 addition & 1 deletion src/test/emmetHelper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Validate Abbreviations', () => {
'div{ a (b) c}',
'div{ a (b) c}+div{ a (( }'
];
const cssAbbreviations = ['#123', '#abc'];
const cssAbbreviations = ['#123', '#abc', 'bd1#s'];
htmlAbbreviations.forEach(abbr => {
assert(isAbbreviationValid('html', abbr), `${abbr} should be treated as valid abbreviation`);
});
Expand Down