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.

Sign up for GitHub

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 - Wrap with abbreviation with live preview #45092

Merged
merged 17 commits into from
Mar 7, 2018
Merged

Emmet - Wrap with abbreviation with live preview #45092

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
39e6a2b
Revert "Fix for Emmet's wrap with abbreviation inserting extra spaces…
Mar 3, 2018
4a566ef
Adding ability for emmet to wrap with abbreviation in real time.
Feb 23, 2018
4947eaf
Fixes to wrap in real time:
Feb 28, 2018
3b9af18
Fixes to a few issues.
Mar 3, 2018
57082a4
Ammend for previous revert
Mar 5, 2018
b855519
Reapplying reverted commit, fixing the bug for this branch's version
Mar 5, 2018
b576c20
Adding multicursor support
Mar 5, 2018
2821c46
Renaming, refactoring and other stuff
Mar 5, 2018
22ca9c4
More refactorings
ramya-rao-a Mar 6, 2018
12674c6
More renaming and refactoring
Mar 6, 2018
0889378
Replacing placeholders when previewing, simplifying the extracting of…
Mar 6, 2018
106bc42
More refactoring
ramya-rao-a Mar 6, 2018
5ea306b
Adding a comment.
Mar 6, 2018
56d3f88
Readding test removed by mistake.
Mar 6, 2018
9fd6348
Refactoring
Mar 7, 2018
70c0a26
Carefully reverting changes in yarn.lock
Mar 7, 2018
206c968
carefully but right
Mar 7, 2018
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
34 changes: 11 additions & 23 deletions extensions/emmet/src/abbreviationActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,14 @@ export function wrapWithAbbreviation(args: any) {
const editor = vscode.window.activeTextEditor;
let rootNode = parseDocument(editor.document, false);

const syntax = getSyntaxFromArgs({ language: editor.document.languageId });
const syntax = getSyntaxFromArgs({ language: editor.document.languageId }) || '';
if (!syntax) {
return;
}

let previewMade = false;

// Fetch general information for the succesive expansions. i.e. the ranges to replace and its contents
let expandAbbrList: ExpandAbbreviationInput[] = [];
let rangesToReplace: PreviewRangesWithContent[] = [];

editor.selections.sort((a: vscode.Selection, b: vscode.Selection) => { return a.start.line - b.start.line; }).forEach(selection => {
Expand Down Expand Up @@ -90,45 +89,34 @@ export function wrapWithAbbreviation(args: any) {

function makeChanges(inputAbbreviation: string | undefined, previewMade?: boolean, definitive?: boolean): Thenable<any> {
if (!inputAbbreviation || !inputAbbreviation.trim() || !helper.isAbbreviationValid(syntax, inputAbbreviation)) {
return previewMade ? revertPreview(editor, rangesToReplace).then(() => { return false; }) : Promise.resolve();
return previewMade ? revertPreview(editor, rangesToReplace) : Promise.resolve();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Returning false allows us to not revert the preview several times. That would not a problem per se, since revertPreview only replaces the current range with the original content, but why make extra unnecessary edits?

}

let extractedResults = helper.extractAbbreviationFromText(inputAbbreviation);
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be an exisitng bug... but shouldnt we check for the validity of the abbreviation after it is extracted instead of before?
Also, if extractedResults is empty then shouldnt we revert the previous preview?

I would suggest to move the extraction to be the first thing that we do in makeChanges(). Then,

if (!extractedResults || !helper.isAbbreviationValid(syntax, extractedResults.abbreviation)) {
return previewMade ? revertPreview(editor, rangesToReplace) : Promise.resolve();
}

if (!extractedResults) {
return Promise.resolve(previewMade);
}
let { abbreviation, filter } = extractedResults;

expandAbbrList = [];

// we need to apply the previewchanges and get the new ranges
let revertPromise: Thenable<any> = Promise.resolve();
let { abbreviation, filter } = extractedResults;
if (definitive) {
if (previewMade) {
revertPromise = revertPreview(editor, rangesToReplace);
}
const revertPromise = previewMade ? revertPreview(editor, rangesToReplace) : Promise.resolve();
return revertPromise.then(() => {
rangesToReplace.forEach(rangesAndContent => {
const expandAbbrList: ExpandAbbreviationInput[] = rangesToReplace.map(rangesAndContent => {
let rangeToReplace = rangesAndContent.originalRange;
let textToWrap = rangeToReplace.isSingleLine ? ['$TM_SELECTED_TEXT'] : ['\n\t$TM_SELECTED_TEXT\n'];
expandAbbrList.push({ syntax: syntax || '', abbreviation, rangeToReplace, textToWrap, filter });
return { syntax, abbreviation, rangeToReplace, textToWrap, filter };
});
return expandAbbreviationInRange(editor, expandAbbrList, true).then(() => { return Promise.resolve(); });
});
} else {
rangesToReplace.forEach(rangesAndContent => {
let match = rangesAndContent.originalContent.match(/\n[\s]*/g);
let textToWrap = match ? ['\n\t' + rangesAndContent.originalContent.split(match[match.length - 1]).join('\n\t') + '\n'] : [rangesAndContent.originalContent];
expandAbbrList.push({ syntax: syntax || '', abbreviation, rangeToReplace: rangesAndContent.originalRange, textToWrap, filter });
});
}

return Promise.resolve().then(() => {
return applyPreview(editor, expandAbbrList, rangesToReplace).then(() => {
return true;
});
const expandAbbrList: ExpandAbbreviationInput[] = rangesToReplace.map(rangesAndContent => {
let match = rangesAndContent.originalContent.match(/\n[\s]*/g);
let textToWrap = match ? ['\n\t' + rangesAndContent.originalContent.split(match[match.length - 1]).join('\n\t') + '\n'] : [rangesAndContent.originalContent];
return { syntax, abbreviation, rangeToReplace: rangesAndContent.originalRange, textToWrap, filter };
});

return applyPreview(editor, expandAbbrList, rangesToReplace);
}

// On inputBox closing
Expand Down