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

[snippets] Fix incorrect range traversal when resolving variables #1043

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
10 changes: 6 additions & 4 deletions packages/snippets/lib/snippet-expansion.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,12 @@ module.exports = class SnippetExpansion {
// markers are crucial for ensuring we adapt to those changes.
for (const variable of this.snippet.variables) {
const {point} = variable
const marker = this.getMarkerLayer(this.editor).markBufferRange([
startPosition.traverse(point),
startPosition.traverse(point)
], {exclusive: false})
let markerRange = new Range(
startPosition.translate(point),
startPosition.translate(point)
)
const marker = this.getMarkerLayer(this.editor).markBufferRange(
markerRange, {exclusive: false})
this.markersForVariables.set(variable, marker)
}
}
Expand Down
9 changes: 9 additions & 0 deletions packages/snippets/spec/.eslintrc
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
{
"env": { "jasmine": true },
"rules": {
"node/no-unpublished-require": "off",
"node/no-extraneous-require": "off",
"no-unused-vars": "off",
"no-empty": "off",
"object-curly-spacing": ["error", "always"],
"semi": ["error", "always"]
},
"globals": {
"waitsForPromise": true
}
}
29 changes: 20 additions & 9 deletions packages/snippets/spec/snippets-spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const path = require('path');
const temp = require('temp').track();
const Snippets = require('../lib/snippets');
const {TextEditor} = require('atom');
const { TextEditor } = require('atom');
const crypto = require('crypto');

const SUPPORTS_UUID = ('randomUUID' in crypto) && (typeof crypto.randomUUID === 'function');
Expand All @@ -14,8 +14,8 @@ describe("Snippets extension", () => {
if (param == null) {
param = {};
}
const {shift} = param;
const event = atom.keymaps.constructor.buildKeydownEvent('tab', {shift, target: editorElement});
const { shift } = param;
const event = atom.keymaps.constructor.buildKeydownEvent('tab', { shift, target: editorElement });
atom.keymaps.handleKeyboardEvent(event);
};

Expand Down Expand Up @@ -113,7 +113,7 @@ describe("Snippets extension", () => {
invalidSnippets = 3;
expect(snippets.getSnippets(editor)).toEqual({});

invalidSnippets = {a: null};
invalidSnippets = { a: null };
expect(snippets.getSnippets(editor)).toEqual({});
});

Expand Down Expand Up @@ -499,14 +499,14 @@ third tabstop $3\
expect(editor.getSelectedBufferRange()).toEqual([[2, 40], [2, 40]]);

// tab backwards
simulateTabKeyEvent({shift: true});
simulateTabKeyEvent({ shift: true });
expect(editor.getSelectedBufferRange()).toEqual([[2, 14], [2, 17]]); // should highlight text typed at tab stop

simulateTabKeyEvent({shift: true});
simulateTabKeyEvent({ shift: true });
expect(editor.getSelectedBufferRange()).toEqual([[3, 15], [3, 15]]);

// shift-tab on first tab-stop does nothing
simulateTabKeyEvent({shift: true});
simulateTabKeyEvent({ shift: true });
expect(editor.getCursorScreenPosition()).toEqual([3, 15]);

// tab through all tab stops, then tab on last stop to terminate snippet
Expand Down Expand Up @@ -575,7 +575,7 @@ third tabstop $3\
simulateTabKeyEvent();

editor.moveRight();
simulateTabKeyEvent({shift: true});
simulateTabKeyEvent({ shift: true });
expect(editor.getCursorBufferPosition()).toEqual([4, 15]);
});
});
Expand Down Expand Up @@ -672,7 +672,7 @@ third tabstop $3\

describe("when the snippet spans multiple lines", () => {
beforeEach(async () => {
editor.update({autoIndent: true});
editor.update({ autoIndent: true });
// editor.update() returns a Promise that never gets resolved, so we
// need to return undefined to avoid a timeout in the spec.
// TODO: Figure out why `editor.update({autoIndent: true})` never gets resolved.
Expand Down Expand Up @@ -1055,6 +1055,17 @@ foo\
editor.insertText('TEST');
expect(editor.getText()).toBe("// TEST\n// ====");
});

describe("and the editor is indented", () => {
it("traverses newlines properly", () => {
editor.setText(' bannerGeneric');
editor.setCursorScreenPosition([0, 15]);
simulateTabKeyEvent();
expect(editor.getText()).toBe(" // \n // ");
editor.insertText('TEST');
expect(editor.getText()).toBe(" // TEST\n // ====");
});
});
});

describe("and the document is HTML", () => {
Expand Down
Loading