Fix edge-case :write issues

nv/ex_cmds.py

@@ -62,12 +62,10 @@
 from NeoVintageous.nv.state import State
 from NeoVintageous.nv.ui import ui_bell
 from NeoVintageous.nv.utils import adding_regions
-from NeoVintageous.nv.utils import get_insertion_point_at_b
 from NeoVintageous.nv.utils import has_dirty_buffers
 from NeoVintageous.nv.utils import has_newline_at_eof
 from NeoVintageous.nv.utils import next_non_blank
 from NeoVintageous.nv.utils import regions_transformer
-from NeoVintageous.nv.utils import replace_sel
 from NeoVintageous.nv.utils import row_at
 from NeoVintageous.nv.utils import set_selection
 from NeoVintageous.nv.vi.settings import get_cache_value
@@ -1238,112 +1236,79 @@ def ex_wqall(window, **kwargs):
 
 
 @_init_cwd
-def ex_write(window, view, file_name, cmd, line_range, forceit=False, **kwargs):
-    if kwargs.get('++') or cmd:
-        return status_message('argument not implemented')
-
-    appends = kwargs.get('>>')
-
-    def _is_file_read_only(fname):
-        if fname:
+def ex_write(window, view, file_name, line_range, forceit=False, **kwargs):
+    def _is_read_only(file_name):
+        if file_name:
             try:
-                return (stat.S_IMODE(os.stat(fname).st_mode) & stat.S_IWUSR != stat.S_IWUSR)
+                return (stat.S_IMODE(os.stat(file_name).st_mode) & stat.S_IWUSR != stat.S_IWUSR)
             except FileNotFoundError:
                 return False
 
         return False
 
-    if appends:
-        def _do_append_to_file(view, file_name, forceit, line_range):
-            r = None
-            if line_range.is_empty:
-                # If the user didn't provide any range data, Vim writes whe whole buffer.
-                r = Region(0, view.size())
-            else:
-                r = line_range.resolve(view)
-
-            if not forceit and not os.path.exists(file_name):
-                return status_message("E212: Can't open file for writing: %s" % file_name)
-
-            try:
-                with open(file_name, 'at') as f:
-                    text = view.substr(r)
-                    f.write(text)
-
-                # TODO: make this `show_info` instead.
-                return status_message('Appended to ' + os.path.abspath(file_name))
-
-            except IOError as e:
-                return status_message('could not write file {}'.format(str(e)))
-
-        def _do_append(view, file_name, forceit, line_range):
-            if file_name:
-                return _do_append_to_file(view, file_name, forceit, line_range)
-
-            r = None
-            if line_range.is_empty:
-                # If the user didn't provide any range data, Vim appends whe whole buffer.
-                r = Region(0, view.size())
-            else:
-                r = line_range.resolve(view)
-
-            text = view.substr(r)
-            text = text if text.startswith('\n') else '\n' + text
+    def _get_buffer(view, line_range):
+        # type: (...) -> str
+        # If no range, write whe whole buffer.
+        if line_range.is_empty:
+            region = Region(0, view.size())
+        else:
+            region = line_range.resolve(view)
 
-            location = get_insertion_point_at_b(view.sel()[0])
+        return view.substr(region)
 
-            view.run_command('append', {'characters': text})
-            view.run_command('save')
+    def _append_to_file(view, file_name, forceit, line_range):
+        if not forceit and not os.path.exists(file_name):
+            return status_message("E212: Can't open file for writing: %s", file_name)
 
-            replace_sel(view, Region(view.line(location).a))
+        try:
+            with open(file_name, 'at') as f:
+                f.write(_get_buffer(view, line_range))
 
-            # TODO [review] State dependency
-            state = State(view)
-            enter_normal_mode(window, state.mode)
-            state.enter_normal_mode()
+            return status_message('Appended to %s' % os.path.abspath(file_name))
+        except IOError as e:
+            return status_message('could not write file %s', str(e))
 
-        return _do_append(view, file_name, forceit, line_range)
+    def _append(view, line_range):
+        view.run_command('append', {'characters': _get_buffer(view, line_range)})
+        view.run_command('save')
 
-    if file_name:
-        def _do_write(window, view, file_name, forceit, line_range):
-            fname = file_name
+        # TODO [review] State dependency
+        enter_normal_mode(window, State(view).mode)
 
-            if not forceit:
-                if os.path.exists(fname):
-                    return ui_bell("E13: File exists (add ! to override)")
+    def _write_to_file(window, view, file_name, forceit, line_range):
+        if not forceit:
+            if os.path.exists(file_name):
+                return ui_bell("E13: File exists (add ! to override)")
 
-                if _is_file_read_only(fname):
-                    return ui_bell("E45: 'readonly' option is set (add ! to override)")
+            if _is_read_only(file_name):
+                return ui_bell("E45: 'readonly' option is set (add ! to override)")
 
-            region = None
-            if line_range.is_empty:
-                # If the user didn't provide any range data, Vim writes whe whole buffer.
-                region = Region(0, view.size())
-            else:
-                region = line_range.resolve(view)
+        try:
+            file_path = os.path.abspath(os.path.expandvars(os.path.expanduser(file_name)))
+            with open(file_path, 'wt') as f:
+                f.write(_get_buffer(view, line_range))
 
-            assert region is not None, "range cannot be None"
+            view.retarget(file_path)
+            window.run_command('save')
+        except IOError:
+            return status_message("E212: Can't open file for writing: {}".format(file_name))
 
-            try:
-                expanded_path = os.path.expandvars(os.path.expanduser(fname))
-                expanded_path = os.path.abspath(expanded_path)
-                with open(expanded_path, 'wt') as f:
-                    text = view.substr(region)
-                    f.write(text)
+    if kwargs.get('++') or kwargs.get('cmd'):
+        return status_message('argument not implemented')
 
-                view.retarget(expanded_path)
-                window.run_command('save')
+    if kwargs.get('>>'):
+        if file_name:
+            return _append_to_file(view, file_name, forceit, line_range)
 
-            except IOError:
-                return status_message("E212: Can't open file for writing: {}".format(fname))
+        return _append(view, line_range)
 
-        return _do_write(window, view, file_name, forceit, line_range)
+    if file_name:
+        return _write_to_file(window, view, file_name, forceit, line_range)
 
     if not view.file_name():
         return status_message("E32: No file name")
 
-    read_only = _is_file_read_only(view.file_name()) or view.is_read_only()
-    if read_only and not forceit:
+    if _is_read_only(view.file_name()) or view.is_read_only() and not forceit:
         return ui_bell("E45: 'readonly' option is set (add ! to override)")
 
     window.run_command('save')

nv/utils.py

@@ -169,15 +169,11 @@ def f(view, s):
 
 def replace_sel(view, new_sel):
     # type: (...) -> None
-    if new_sel is None or new_sel == []:
-        raise ValueError('no new_sel')
-
     view.sel().clear()
     if isinstance(new_sel, list):
         view.sel().add_all(new_sel)
-        return
-
-    view.sel().add(new_sel)
+    else:
+        view.sel().add(new_sel)
 
 
 def get_insertion_point_at_b(region):

tests/functional/test__ex_write.py

@@ -55,26 +55,42 @@ def test_write(self):
             self.feed(':write >>')
             self.assertEqual(file_name, self.view.file_name())
             with open(file_name, 'r') as f:
-                self.assertEqual('buzz\n\nbuzz\n', f.read())
+                self.assertEqual('buzz\nbuzz\n', f.read())
 
             # Test can write to another file.
             self.feed(':write ' + file_name_alt)
             self.assertEqual(file_name_alt, self.view.file_name())
             with open(file_name_alt, 'r') as f:
-                self.assertEqual('buzz\n\nbuzz\n', f.read())
+                self.assertEqual('buzz\nbuzz\n', f.read())
 
             self.assertNoStatusMessage()
 
+            # Test can append to file.
             self.feed(':write >> ' + file_name)
             self.assertStatusMessage('Appended to %s' % file_name)
             self.assertEqual(file_name_alt, self.view.file_name())
             with open(file_name_alt, 'r') as f:
-                self.assertEqual('buzz\n\nbuzz\n', f.read())
+                self.assertEqual('buzz\nbuzz\n', f.read())
             with open(file_name, 'r') as f:
-                self.assertEqual('buzz\n\nbuzz\nbuzz\n\nbuzz\n', f.read())
+                self.assertEqual('buzz\nbuzz\nbuzz\nbuzz\n', f.read())
 
+            # Test trying to append to non-existing file.
             self.feed(':write >> ' + file_name_noop)
             self.assertStatusMessage('E212: Can\'t open file for writing: %s' % file_name_noop, count=2)
 
+            # Test force append to non-existing file.
             self.feed(':write! >> ' + file_name_noop)
             self.assertStatusMessage('Appended to %s' % file_name_noop, count=3)
+
+            # Test appending from a line range.
+            self.assertEqual(file_name_alt, self.view.file_name())
+            self.normal('1\n2\n3\n4\n5\n6\n7\n8\n9\n0\n')
+            self.feed(':write')
+            self.feed(':3,6write >> ' + file_name_alt)
+            self.assertStatusMessage('Appended to %s' % file_name_alt, count=4)
+            with open(file_name_alt, 'r') as f:
+                self.assertEqual('1\n2\n3\n4\n5\n6\n7\n8\n9\n0\n3\n4\n5\n6\n', f.read())
+
+            # # Test trying to write to a file that already exists.
+            # self.feed(':write ' + file_name)
+            # self.assertStatusMessage('E13: File exists (add ! to override)', count=5)

tests/unittest.py

@@ -178,6 +178,7 @@ def assertNotSetting(self, name):
 
     def set_option(self, name, value):
         _set_option(self.view, name, value)
+        # Options via settings is DEPRECATED
         self.settings().set('vintageous_%s' % name, value)
 
     def assertOption(self, name, expected, msg=None):
@@ -223,12 +224,12 @@ def _setupView(self, text, mode, reverse=False, vblock_direction=None):
                 else:
                     self.view.sel().add_all(v_sels)
 
-            # This is required, because the cursor in VISUAL mode is a block
+            # This is required because the cursor in VISUAL mode is a block
             # cursor. Without this setting some tests will pass when the window
-            # running the tests has focus, and fail when it doesn't have focus.
-            # This happens because ST doesn't fire events for views (test views)
-            # when the window loses focus: the on_activated() event fixes VISUAL
-            # mode selections that don't have a correct caret state.
+            # running the tests has focus and fail when it doesn't have focus.
+            # This happens because Sublime doesn't fire events for views when
+            # the window loses focus (usually the on_activated() event fixes
+            # VISUAL mode selections that don't have a correct cursor state).
             self.view.settings().set('inverse_caret_state', True)
 
             if mode == VISUAL_BLOCK:
@@ -1102,7 +1103,9 @@ def _visible_region():
 
 
 def mock_run_commands(*methods):
-    """Mock command runners.
+    """Mock commands.
+
+    Useful to mock builtin Sublime Text commands.
 
     Usage: