From c863cbeac809cf0d8d1e66e55f0b0557e96b3078 Mon Sep 17 00:00:00 2001 From: Joseph Frazier <1212jtraceur@gmail.com> Date: Mon, 5 Jun 2017 21:51:53 -0400 Subject: [PATCH] Translate cursor relative to nearest node, not SourceElement (#1989) * Add test demonstrating SourceElement-relative cursor translation See https://github.com/prettier/prettier/issues/1981#issuecomment-306326739 * Translate cursor relative to nearest node, not SourceElement This partially address https://github.com/prettier/prettier/issues/1981 See https://github.com/prettier/prettier/issues/1981#issuecomment-306326739 * Add test demonstrating incorrect cursor translation Since the `cursorOffset` option (introduced in #1637) works by tracking the cursor position relative to an AST node (rather than a CST token), it can produce incorrect results. See https://github.com/prettier/prettier/issues/1981 --- index.js | 18 ++++++++++++++---- tests/cursor/jsfmt.spec.js | 16 ++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 8c2cd9039..e95465356 100644 --- a/index.js +++ b/index.js @@ -125,7 +125,8 @@ function findSiblingAncestors(startNodeAndParents, endNodeAndParents) { }; } -function findNodeAtOffset(node, offset, parentNodes) { +function findNodeAtOffset(node, offset, predicate, parentNodes) { + predicate = predicate || (() => true); parentNodes = parentNodes || []; const start = util.locStart(node); const end = util.locEnd(node); @@ -134,6 +135,7 @@ function findNodeAtOffset(node, offset, parentNodes) { const childResult = findNodeAtOffset( childNode, offset, + predicate, [node].concat(parentNodes) ); if (childResult) { @@ -141,7 +143,7 @@ function findNodeAtOffset(node, offset, parentNodes) { } } - if (isSourceElement(node)) { + if (predicate(node)) { return { node: node, parentNodes: parentNodes @@ -199,8 +201,16 @@ function calculateRange(text, opts, ast) { } } - const startNodeAndParents = findNodeAtOffset(ast, startNonWhitespace); - const endNodeAndParents = findNodeAtOffset(ast, endNonWhitespace); + const startNodeAndParents = findNodeAtOffset( + ast, + startNonWhitespace, + isSourceElement + ); + const endNodeAndParents = findNodeAtOffset( + ast, + endNonWhitespace, + isSourceElement + ); const siblingAncestors = findSiblingAncestors( startNodeAndParents, endNodeAndParents diff --git a/tests/cursor/jsfmt.spec.js b/tests/cursor/jsfmt.spec.js index 090654b16..e5a0dc39d 100644 --- a/tests/cursor/jsfmt.spec.js +++ b/tests/cursor/jsfmt.spec.js @@ -6,3 +6,19 @@ test("translates cursor correctly in basic case", () => { cursorOffset: 1 }); }); + +test("positions cursor relative to closest node, not SourceElement", () => { + const code = "return 15"; + expect(prettier.formatWithCursor(code, { cursorOffset: 15 })).toEqual({ + formatted: "return 15;\n", + cursorOffset: 7 + }); +}); + +test("keeps cursor inside formatted node", () => { + const code = "return 15"; + expect(prettier.formatWithCursor(code, { cursorOffset: 14 })).toEqual({ + formatted: "return 15;\n", + cursorOffset: 14 // TODO fix this + }); +});