Repository: prettier/prettier-atom
Pull request #333 (2017-12-31 04:28:24)

fix(set-text-via-diff): base use on option instead of flaky buffer range comparison
We were determining whether or not to use setTextViaDiff (more performant) versus the normal
setTextInBufferRange (less performant but allows replacing only a subset of text in a file if
desired) by whether or not the buffer range of the file equaled the buffer range of the text to be
transformed. However, in some cases such as when the editor automatically inserts an ending newline,
these two ranges can become unequal and we revert to using setTextInBufferRange. This is less
performant and not really necessary. The fix is to just pass an option when calling
`executePrettierOnBufferRange` from `formatOnSave`.


Patch:
diff --git a/dist/executePrettier/executePrettierOnBufferRange.js b/dist/executePrettier/executePrettierOnBufferRange.js
index bdcc3e1..ea88623 100644
--- a/dist/executePrettier/executePrettierOnBufferRange.js
+++ b/dist/executePrettier/executePrettierOnBufferRange.js
@@ -99,8 +99,8 @@ var executePrettierOrIntegration = function () {
 }();
 
 var executePrettierOnBufferRange = function () {
-  var _ref2 = (0, _asyncToGenerator3.default)(_regenerator2.default.mark(function _callee2(editor, bufferRange) {
-    var cursorPositionPriorToFormat, textToTransform, transformed, isTextUnchanged, editorBuffer;
+  var _ref2 = (0, _asyncToGenerator3.default)(_regenerator2.default.mark(function _callee2(editor, bufferRange, options) {
+    var cursorPositionPriorToFormat, textToTransform, transformed, isTextUnchanged;
     return _regenerator2.default.wrap(function _callee2$(_context2) {
       while (1) {
         switch (_context2.prev = _context2.next) {
@@ -132,12 +132,12 @@ var executePrettierOnBufferRange = function () {
 
           case 11:
 
-            // we use setTextViaDiff when formatting the entire buffer to improve performance,
-            // maintain metadata (bookmarks, folds, etc) and eliminate syntax highlight flickering
-            editorBuffer = editor.getBuffer();
-
-            if (editorBuffer.getRange().isEqual(bufferRange)) {
-              editorBuffer.setTextViaDiff(transformed);
+            if (options.setTextViaDiff) {
+              // we use setTextViaDiff when formatting the entire buffer to improve performance,
+              // maintain metadata (bookmarks, folds, etc) and eliminate syntax highlight flickering
+              // however, we can't always use it because it replaces all text in the file and sometimes
+              // we're only editing a sub-selection of the text in a file
+              editor.getBuffer().setTextViaDiff(transformed);
             } else {
               editor.setTextInBufferRange(bufferRange, transformed);
             }
@@ -145,7 +145,7 @@ var executePrettierOnBufferRange = function () {
             editor.setCursorScreenPosition(cursorPositionPriorToFormat);
             runLinter(editor);
 
-          case 15:
+          case 14:
           case 'end':
             return _context2.stop();
         }
@@ -153,7 +153,7 @@ var executePrettierOnBufferRange = function () {
     }, _callee2, undefined);
   }));
 
-  return function executePrettierOnBufferRange(_x3, _x4) {
+  return function executePrettierOnBufferRange(_x3, _x4, _x5) {
     return _ref2.apply(this, arguments);
   };
 }();
diff --git a/dist/formatOnSave/index.js b/dist/formatOnSave/index.js
index 6f24799..b00eceb 100644
--- a/dist/formatOnSave/index.js
+++ b/dist/formatOnSave/index.js
@@ -19,7 +19,7 @@ var _require4 = require('../atomInterface'),
 var shouldFormatOnSave = require('./shouldFormatOnSave');
 
 var callAppropriatePrettierExecutor = function callAppropriatePrettierExecutor(editor) {
-  return isCurrentScopeEmbeddedScope(editor) ? executePrettierOnEmbeddedScripts(editor) : executePrettierOnBufferRange(editor, getBufferRange(editor));
+  return isCurrentScopeEmbeddedScope(editor) ? executePrettierOnEmbeddedScripts(editor) : executePrettierOnBufferRange(editor, getBufferRange(editor), { setTextViaDiff: true });
 };
 
 var formatOnSaveIfAppropriate = _.flow(_.tap(clearLinterErrors), _.cond([[shouldFormatOnSave, callAppropriatePrettierExecutor]]));
diff --git a/src/executePrettier/executePrettierOnBufferRange.js b/src/executePrettier/executePrettierOnBufferRange.js
index c71ee85..115bb5c 100644
--- a/src/executePrettier/executePrettierOnBufferRange.js
+++ b/src/executePrettier/executePrettierOnBufferRange.js
@@ -38,7 +38,11 @@ const executePrettierOrIntegration = async (editor: TextEditor, text: string) =>
   }
 };
 
-const executePrettierOnBufferRange = async (editor: TextEditor, bufferRange: Range) => {
+const executePrettierOnBufferRange = async (
+  editor: TextEditor,
+  bufferRange: Range,
+  options?: { setTextViaDiff?: boolean },
+) => {
   const cursorPositionPriorToFormat = editor.getCursorScreenPosition();
   const textToTransform = editor.getTextInBufferRange(bufferRange);
   const transformed = await executePrettierOrIntegration(editor, textToTransform);
@@ -51,11 +55,12 @@ const executePrettierOnBufferRange = async (editor: TextEditor, bufferRange: Ran
     return;
   }
 
-  // we use setTextViaDiff when formatting the entire buffer to improve performance,
-  // maintain metadata (bookmarks, folds, etc) and eliminate syntax highlight flickering
-  const editorBuffer = editor.getBuffer();
-  if (editorBuffer.getRange().isEqual(bufferRange)) {
-    editorBuffer.setTextViaDiff(transformed);
+  if (options && options.setTextViaDiff) {
+    // we use setTextViaDiff when formatting the entire buffer to improve performance,
+    // maintain metadata (bookmarks, folds, etc) and eliminate syntax highlight flickering
+    // however, we can't always use it because it replaces all text in the file and sometimes
+    // we're only editing a sub-selection of the text in a file
+    editor.getBuffer().setTextViaDiff(transformed);
   } else {
     editor.setTextInBufferRange(bufferRange, transformed);
   }
diff --git a/src/formatOnSave/index.js b/src/formatOnSave/index.js
index 8ff0380..a13527f 100644
--- a/src/formatOnSave/index.js
+++ b/src/formatOnSave/index.js
@@ -9,7 +9,7 @@ const shouldFormatOnSave = require('./shouldFormatOnSave');
 const callAppropriatePrettierExecutor = (editor: TextEditor) =>
   isCurrentScopeEmbeddedScope(editor)
     ? executePrettierOnEmbeddedScripts(editor)
-    : executePrettierOnBufferRange(editor, getBufferRange(editor));
+    : executePrettierOnBufferRange(editor, getBufferRange(editor), { setTextViaDiff: true });
 
 const formatOnSaveIfAppropriate: TextEditor => void = _.flow(
   _.tap(clearLinterErrors),


Test patch:
diff --git a/src/executePrettier/executePrettierOnBufferRange.test.js b/src/executePrettier/executePrettierOnBufferRange.test.js
index e2cfc62..a714957 100644
--- a/src/executePrettier/executePrettierOnBufferRange.test.js
+++ b/src/executePrettier/executePrettierOnBufferRange.test.js
@@ -40,14 +40,14 @@ it('sets the transformed text in the buffer range', async () => {
   expect(editor.setTextInBufferRange).toHaveBeenCalledWith(bufferRangeFixture, 'const foo = 2;');
 });
 
-it('sets the transformed text via diff when buffer equals entire range of editor', async () => {
+it('sets the transformed text via diff when the option is passed', async () => {
   const setTextViaDiffMock = jest.fn();
   editor.getBuffer.mockImplementation(() => ({
     getRange: () => ({ isEqual: () => true }),
     setTextViaDiff: setTextViaDiffMock,
   }));
 
-  await executePrettierOnBufferRange(editor, bufferRangeFixture);
+  await executePrettierOnBufferRange(editor, bufferRangeFixture, { setTextViaDiff: true });
 
   expect(prettier.format).toHaveBeenCalledWith('const foo = (2);', { useTabs: false });
   expect(setTextViaDiffMock).toHaveBeenCalledWith('const foo = 2;');
diff --git a/src/formatOnSave/index.test.js b/src/formatOnSave/index.test.js
index 487c271..d1d6262 100644
--- a/src/formatOnSave/index.test.js
+++ b/src/formatOnSave/index.test.js
@@ -27,7 +27,7 @@ it('executes prettier on the buffer range if appropriate and scope is not embedd
 
   formatOnSave(editor);
 
-  expect(executePrettierOnBufferRange).toHaveBeenCalledWith(editor, mockRange);
+  expect(executePrettierOnBufferRange).toHaveBeenCalledWith(editor, mockRange, { setTextViaDiff: true });
 });
 
 it('executes prettier on the embedded scripts if appropriate and scope is embedded', () => {